* [PATCH 1/2] drm/amdgpu/mes11: fix set_hw_resources_1 calculation @ 2025-02-07 14:43 Alex Deucher 2025-02-07 14:43 ` [PATCH V3 2/2] drm/amdgpu/mes: Add cleaner shader fence address handling in MES for GFX11 Alex Deucher 0 siblings, 1 reply; 6+ messages in thread From: Alex Deucher @ 2025-02-07 14:43 UTC (permalink / raw) To: amd-gfx; +Cc: Alex Deucher It's GPU page size not CPU page size. In most cases they are the same, but not always. This can lead to overallocation on systems with larger pages. Signed-off-by: Alex Deucher <alexander.deucher@amd.com> --- drivers/gpu/drm/amd/amdgpu/mes_v11_0.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c b/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c index bf51f3dcc130e..e862a3febe2b2 100644 --- a/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c +++ b/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c @@ -743,7 +743,7 @@ static int mes_v11_0_set_hw_resources(struct amdgpu_mes *mes) static int mes_v11_0_set_hw_resources_1(struct amdgpu_mes *mes) { - int size = 128 * PAGE_SIZE; + int size = 128 * AMDGPU_GPU_PAGE_SIZE; int ret = 0; struct amdgpu_device *adev = mes->adev; union MESAPI_SET_HW_RESOURCES_1 mes_set_hw_res_pkt; -- 2.48.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH V3 2/2] drm/amdgpu/mes: Add cleaner shader fence address handling in MES for GFX11 2025-02-07 14:43 [PATCH 1/2] drm/amdgpu/mes11: fix set_hw_resources_1 calculation Alex Deucher @ 2025-02-07 14:43 ` Alex Deucher 2025-02-07 14:50 ` Christian König 0 siblings, 1 reply; 6+ messages in thread From: Alex Deucher @ 2025-02-07 14:43 UTC (permalink / raw) To: amd-gfx Cc: Srinivasan Shanmugam, lin cao, Jingwen Chen, Christian König, Alex Deucher, Shaoyun Liu From: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com> 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. This is done by calculating the address using the write-back memory base address and the cleaner fence offset. Cc: lin cao <lin.cao@amd.com> Cc: Jingwen Chen <Jingwen.Chen2@amd.com> Cc: Christian König <christian.koenig@amd.com> Cc: Alex Deucher <alexander.deucher@amd.com> Suggested-by: Shaoyun Liu <shaoyun.liu@amd.com> Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com> Signed-off-by: Alex Deucher <alexander.deucher@amd.com> --- drivers/gpu/drm/amd/amdgpu/mes_v11_0.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c b/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c index e862a3febe2b2..e22d0ee6d8a3a 100644 --- a/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c +++ b/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c @@ -754,7 +754,7 @@ static int mes_v11_0_set_hw_resources_1(struct amdgpu_mes *mes) mes_set_hw_res_pkt.header.dwsize = API_FRAME_SIZE_IN_DWORDS; mes_set_hw_res_pkt.enable_mes_info_ctx = 1; - ret = amdgpu_bo_create_kernel(adev, size, PAGE_SIZE, + ret = amdgpu_bo_create_kernel(adev, size + AMDGPU_GPU_PAGE_SIZE, PAGE_SIZE, AMDGPU_GEM_DOMAIN_VRAM, &mes->resource_1, &mes->resource_1_gpu_addr, @@ -765,7 +765,10 @@ static int mes_v11_0_set_hw_resources_1(struct amdgpu_mes *mes) } mes_set_hw_res_pkt.mes_info_ctx_mc_addr = mes->resource_1_gpu_addr; - mes_set_hw_res_pkt.mes_info_ctx_size = mes->resource_1->tbo.base.size; + mes_set_hw_res_pkt.mes_info_ctx_size = size; + mes_set_hw_res_pkt.cleaner_shader_fence_mc_addr = + mes->resource_1_gpu_addr + size; + return mes_v11_0_submit_pkt_and_poll_completion(mes, &mes_set_hw_res_pkt, sizeof(mes_set_hw_res_pkt), offsetof(union MESAPI_SET_HW_RESOURCES_1, api_status)); @@ -1632,12 +1635,10 @@ static int mes_v11_0_hw_init(struct amdgpu_ip_block *ip_block) if (r) goto failure; - if (amdgpu_sriov_is_mes_info_enable(adev)) { - r = mes_v11_0_set_hw_resources_1(&adev->mes); - if (r) { - DRM_ERROR("failed mes_v11_0_set_hw_resources_1, r=%d\n", r); - goto failure; - } + r = mes_v11_0_set_hw_resources_1(&adev->mes); + if (r) { + DRM_ERROR("failed mes_v11_0_set_hw_resources_1, r=%d\n", r); + goto failure; } r = mes_v11_0_query_sched_status(&adev->mes); @@ -1665,10 +1666,9 @@ static int mes_v11_0_hw_init(struct amdgpu_ip_block *ip_block) static int mes_v11_0_hw_fini(struct amdgpu_ip_block *ip_block) { struct amdgpu_device *adev = ip_block->adev; - if (amdgpu_sriov_is_mes_info_enable(adev)) { - amdgpu_bo_free_kernel(&adev->mes.resource_1, &adev->mes.resource_1_gpu_addr, - &adev->mes.resource_1_addr); - } + + amdgpu_bo_free_kernel(&adev->mes.resource_1, &adev->mes.resource_1_gpu_addr, + &adev->mes.resource_1_addr); return 0; } -- 2.48.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH V3 2/2] drm/amdgpu/mes: Add cleaner shader fence address handling in MES for GFX11 2025-02-07 14:43 ` [PATCH V3 2/2] drm/amdgpu/mes: Add cleaner shader fence address handling in MES for GFX11 Alex Deucher @ 2025-02-07 14:50 ` Christian König 2025-02-07 15:23 ` Liu, Shaoyun 0 siblings, 1 reply; 6+ messages in thread From: Christian König @ 2025-02-07 14:50 UTC (permalink / raw) To: Alex Deucher, amd-gfx Cc: Srinivasan Shanmugam, lin cao, Jingwen Chen, Shaoyun Liu Am 07.02.25 um 15:43 schrieb Alex Deucher: > From: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com> > > 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. This is done by calculating the address > using the write-back memory base address and the cleaner fence offset. > > Cc: lin cao <lin.cao@amd.com> > Cc: Jingwen Chen <Jingwen.Chen2@amd.com> > Cc: Christian König <christian.koenig@amd.com> > Cc: Alex Deucher <alexander.deucher@amd.com> > Suggested-by: Shaoyun Liu <shaoyun.liu@amd.com> > Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com> > Signed-off-by: Alex Deucher <alexander.deucher@amd.com> Yeah that makes much more sense. I'm really wondering why the MES hasn't allocated that in it's own memory in the first place? Does the MES expect the kernel driver to wait for that fence value? Anyway Reviewed-by: Christian König <christian.koenig@amd.com> for now. Regards, Christian. > --- > drivers/gpu/drm/amd/amdgpu/mes_v11_0.c | 24 ++++++++++++------------ > 1 file changed, 12 insertions(+), 12 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c b/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c > index e862a3febe2b2..e22d0ee6d8a3a 100644 > --- a/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c > +++ b/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c > @@ -754,7 +754,7 @@ static int mes_v11_0_set_hw_resources_1(struct amdgpu_mes *mes) > mes_set_hw_res_pkt.header.dwsize = API_FRAME_SIZE_IN_DWORDS; > mes_set_hw_res_pkt.enable_mes_info_ctx = 1; > > - ret = amdgpu_bo_create_kernel(adev, size, PAGE_SIZE, > + ret = amdgpu_bo_create_kernel(adev, size + AMDGPU_GPU_PAGE_SIZE, PAGE_SIZE, > AMDGPU_GEM_DOMAIN_VRAM, > &mes->resource_1, > &mes->resource_1_gpu_addr, > @@ -765,7 +765,10 @@ static int mes_v11_0_set_hw_resources_1(struct amdgpu_mes *mes) > } > > mes_set_hw_res_pkt.mes_info_ctx_mc_addr = mes->resource_1_gpu_addr; > - mes_set_hw_res_pkt.mes_info_ctx_size = mes->resource_1->tbo.base.size; > + mes_set_hw_res_pkt.mes_info_ctx_size = size; > + mes_set_hw_res_pkt.cleaner_shader_fence_mc_addr = > + mes->resource_1_gpu_addr + size; > + > return mes_v11_0_submit_pkt_and_poll_completion(mes, > &mes_set_hw_res_pkt, sizeof(mes_set_hw_res_pkt), > offsetof(union MESAPI_SET_HW_RESOURCES_1, api_status)); > @@ -1632,12 +1635,10 @@ static int mes_v11_0_hw_init(struct amdgpu_ip_block *ip_block) > if (r) > goto failure; > > - if (amdgpu_sriov_is_mes_info_enable(adev)) { > - r = mes_v11_0_set_hw_resources_1(&adev->mes); > - if (r) { > - DRM_ERROR("failed mes_v11_0_set_hw_resources_1, r=%d\n", r); > - goto failure; > - } > + r = mes_v11_0_set_hw_resources_1(&adev->mes); > + if (r) { > + DRM_ERROR("failed mes_v11_0_set_hw_resources_1, r=%d\n", r); > + goto failure; > } > > r = mes_v11_0_query_sched_status(&adev->mes); > @@ -1665,10 +1666,9 @@ static int mes_v11_0_hw_init(struct amdgpu_ip_block *ip_block) > static int mes_v11_0_hw_fini(struct amdgpu_ip_block *ip_block) > { > struct amdgpu_device *adev = ip_block->adev; > - if (amdgpu_sriov_is_mes_info_enable(adev)) { > - amdgpu_bo_free_kernel(&adev->mes.resource_1, &adev->mes.resource_1_gpu_addr, > - &adev->mes.resource_1_addr); > - } > + > + amdgpu_bo_free_kernel(&adev->mes.resource_1, &adev->mes.resource_1_gpu_addr, > + &adev->mes.resource_1_addr); > return 0; > } > ^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH V3 2/2] drm/amdgpu/mes: Add cleaner shader fence address handling in MES for GFX11 2025-02-07 14:50 ` Christian König @ 2025-02-07 15:23 ` Liu, Shaoyun 0 siblings, 0 replies; 6+ messages in thread From: Liu, Shaoyun @ 2025-02-07 15:23 UTC (permalink / raw) To: Koenig, Christian, Deucher, Alexander, amd-gfx@lists.freedesktop.org Cc: SHANMUGAM, SRINIVASAN, cao, lin, Chen, JingWen (Wayne) [AMD Official Use Only - AMD Internal Distribution Only] I'm not sure why there is no mes.resource_1_gpu_addr for mes v12 , probably sriov team still not support it . But we do need this cleaner_fence_gpu_addr for both mes v11 and v12 . What MES need is a GPU addr it can update a 32 bit value on it ,an extra page do seems a overkill for this. I still think allcoate a WB in amdgpu_mes_init and free it in amdgpu_mes_fini is a better solution and it can be generic for v11 and v12 . The original design is for windows , and I think they need to sync between their KMD driver and FW for the cleaner shader submission, so the KMD need to be able to check the fence . Regards Shaoyun.liu -----Original Message----- From: Koenig, Christian <Christian.Koenig@amd.com> Sent: Friday, February 7, 2025 9:51 AM To: Deucher, Alexander <Alexander.Deucher@amd.com>; amd-gfx@lists.freedesktop.org Cc: SHANMUGAM, SRINIVASAN <SRINIVASAN.SHANMUGAM@amd.com>; cao, lin <lin.cao@amd.com>; Chen, JingWen (Wayne) <JingWen.Chen2@amd.com>; Liu, Shaoyun <Shaoyun.Liu@amd.com> Subject: Re: [PATCH V3 2/2] drm/amdgpu/mes: Add cleaner shader fence address handling in MES for GFX11 Am 07.02.25 um 15:43 schrieb Alex Deucher: > From: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com> > > 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. This is done by calculating the address > using the write-back memory base address and the cleaner fence offset. > > Cc: lin cao <lin.cao@amd.com> > Cc: Jingwen Chen <Jingwen.Chen2@amd.com> > Cc: Christian König <christian.koenig@amd.com> > Cc: Alex Deucher <alexander.deucher@amd.com> > Suggested-by: Shaoyun Liu <shaoyun.liu@amd.com> > Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com> > Signed-off-by: Alex Deucher <alexander.deucher@amd.com> Yeah that makes much more sense. I'm really wondering why the MES hasn't allocated that in it's own memory in the first place? Does the MES expect the kernel driver to wait for that fence value? Anyway Reviewed-by: Christian König <christian.koenig@amd.com> for now. Regards, Christian. > --- > drivers/gpu/drm/amd/amdgpu/mes_v11_0.c | 24 ++++++++++++------------ > 1 file changed, 12 insertions(+), 12 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c > b/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c > index e862a3febe2b2..e22d0ee6d8a3a 100644 > --- a/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c > +++ b/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c > @@ -754,7 +754,7 @@ static int mes_v11_0_set_hw_resources_1(struct amdgpu_mes *mes) > mes_set_hw_res_pkt.header.dwsize = API_FRAME_SIZE_IN_DWORDS; > mes_set_hw_res_pkt.enable_mes_info_ctx = 1; > > - ret = amdgpu_bo_create_kernel(adev, size, PAGE_SIZE, > + ret = amdgpu_bo_create_kernel(adev, size + AMDGPU_GPU_PAGE_SIZE, > +PAGE_SIZE, > AMDGPU_GEM_DOMAIN_VRAM, > &mes->resource_1, > &mes->resource_1_gpu_addr, > @@ -765,7 +765,10 @@ static int mes_v11_0_set_hw_resources_1(struct amdgpu_mes *mes) > } > > mes_set_hw_res_pkt.mes_info_ctx_mc_addr = mes->resource_1_gpu_addr; > - mes_set_hw_res_pkt.mes_info_ctx_size = mes->resource_1->tbo.base.size; > + mes_set_hw_res_pkt.mes_info_ctx_size = size; > + mes_set_hw_res_pkt.cleaner_shader_fence_mc_addr = > + mes->resource_1_gpu_addr + size; > + > return mes_v11_0_submit_pkt_and_poll_completion(mes, > &mes_set_hw_res_pkt, sizeof(mes_set_hw_res_pkt), > offsetof(union MESAPI_SET_HW_RESOURCES_1, api_status)); @@ > -1632,12 +1635,10 @@ static int mes_v11_0_hw_init(struct amdgpu_ip_block *ip_block) > if (r) > goto failure; > > - if (amdgpu_sriov_is_mes_info_enable(adev)) { > - r = mes_v11_0_set_hw_resources_1(&adev->mes); > - if (r) { > - DRM_ERROR("failed mes_v11_0_set_hw_resources_1, r=%d\n", r); > - goto failure; > - } > + r = mes_v11_0_set_hw_resources_1(&adev->mes); > + if (r) { > + DRM_ERROR("failed mes_v11_0_set_hw_resources_1, r=%d\n", r); > + goto failure; > } > > r = mes_v11_0_query_sched_status(&adev->mes); > @@ -1665,10 +1666,9 @@ static int mes_v11_0_hw_init(struct amdgpu_ip_block *ip_block) > static int mes_v11_0_hw_fini(struct amdgpu_ip_block *ip_block) > { > struct amdgpu_device *adev = ip_block->adev; > - if (amdgpu_sriov_is_mes_info_enable(adev)) { > - amdgpu_bo_free_kernel(&adev->mes.resource_1, &adev->mes.resource_1_gpu_addr, > - &adev->mes.resource_1_addr); > - } > + > + amdgpu_bo_free_kernel(&adev->mes.resource_1, &adev->mes.resource_1_gpu_addr, > + &adev->mes.resource_1_addr); > return 0; > } > ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] drm/amdgpu/mes11: fix set_hw_resources_1 calculation @ 2025-02-10 16:27 Alex Deucher 2025-02-10 19:41 ` Christian König 0 siblings, 1 reply; 6+ messages in thread From: Alex Deucher @ 2025-02-10 16:27 UTC (permalink / raw) To: amd-gfx; +Cc: Alex Deucher, Srinivasan Shanmugam, Christian König It's GPU page size not CPU page size. In most cases they are the same, but not always. This can lead to overallocation on systems with larger pages. Cc: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com> Cc: Christian König <christian.koenig@amd.com> Signed-off-by: Alex Deucher <alexander.deucher@amd.com> --- drivers/gpu/drm/amd/amdgpu/mes_v11_0.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c b/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c index bf51f3dcc130e..e862a3febe2b2 100644 --- a/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c +++ b/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c @@ -743,7 +743,7 @@ static int mes_v11_0_set_hw_resources(struct amdgpu_mes *mes) static int mes_v11_0_set_hw_resources_1(struct amdgpu_mes *mes) { - int size = 128 * PAGE_SIZE; + int size = 128 * AMDGPU_GPU_PAGE_SIZE; int ret = 0; struct amdgpu_device *adev = mes->adev; union MESAPI_SET_HW_RESOURCES_1 mes_set_hw_res_pkt; -- 2.48.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] drm/amdgpu/mes11: fix set_hw_resources_1 calculation 2025-02-10 16:27 [PATCH 1/2] drm/amdgpu/mes11: fix set_hw_resources_1 calculation Alex Deucher @ 2025-02-10 19:41 ` Christian König 0 siblings, 0 replies; 6+ messages in thread From: Christian König @ 2025-02-10 19:41 UTC (permalink / raw) To: Alex Deucher, amd-gfx; +Cc: Srinivasan Shanmugam Am 10.02.25 um 17:27 schrieb Alex Deucher: > It's GPU page size not CPU page size. In most cases they > are the same, but not always. This can lead to overallocation > on systems with larger pages. > > Cc: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com> > Cc: Christian König <christian.koenig@amd.com> > Signed-off-by: Alex Deucher <alexander.deucher@amd.com> Reviewed-by: Christian König <christian.koenig@amd.com> > --- > drivers/gpu/drm/amd/amdgpu/mes_v11_0.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c b/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c > index bf51f3dcc130e..e862a3febe2b2 100644 > --- a/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c > +++ b/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c > @@ -743,7 +743,7 @@ static int mes_v11_0_set_hw_resources(struct amdgpu_mes *mes) > > static int mes_v11_0_set_hw_resources_1(struct amdgpu_mes *mes) > { > - int size = 128 * PAGE_SIZE; > + int size = 128 * AMDGPU_GPU_PAGE_SIZE; > int ret = 0; > struct amdgpu_device *adev = mes->adev; > union MESAPI_SET_HW_RESOURCES_1 mes_set_hw_res_pkt; ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-02-10 19:41 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-02-07 14:43 [PATCH 1/2] drm/amdgpu/mes11: fix set_hw_resources_1 calculation Alex Deucher 2025-02-07 14:43 ` [PATCH V3 2/2] drm/amdgpu/mes: Add cleaner shader fence address handling in MES for GFX11 Alex Deucher 2025-02-07 14:50 ` Christian König 2025-02-07 15:23 ` Liu, Shaoyun -- strict thread matches above, loose matches on Subject: below -- 2025-02-10 16:27 [PATCH 1/2] drm/amdgpu/mes11: fix set_hw_resources_1 calculation Alex Deucher 2025-02-10 19:41 ` Christian König
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.