<!DOCTYPE html><html><head> <meta http-equiv="Content-Type" content="text/html; charset=utf-8"> </head> <body> <div class="moz-cite-prefix">On 1/14/2025 1:52 PM, Elena Sakhnovitch wrote: </div> <blockquote type="cite" cite="mid:20250114195251.42661-1-Elena.Sakhnovitch@amd.com"> <pre wrap="" class="moz-quote-pre">From: Harish Kasiviswanathan <a class="moz-txt-link-rfc2396E" href="mailto:Harish.Kasiviswanathan@amd.com"><Harish.Kasiviswanathan@amd.com></a> Currently, grace period is modified only for gfx943 APU. In the future this might need to be set for other ASICs too. Either ways, asic specific values should be handled by asic specific functions. Signed-off-by: Harish Kasiviswanathan <a class="moz-txt-link-rfc2396E" href="mailto:Harish.Kasiviswanathan@amd.com"><Harish.Kasiviswanathan@amd.com></a> Signed-off-by: Elena Sakhnovitch <a class="moz-txt-link-rfc2396E" href="mailto:Elena.Sakhnovitch@amd.com"><Elena.Sakhnovitch@amd.com></a> --- .../drm/amd/amdkfd/kfd_device_queue_manager.c | 24 ++++++------------- .../drm/amd/amdkfd/kfd_device_queue_manager.h | 3 ++- .../gpu/drm/amd/amdkfd/kfd_packet_manager.c | 3 +++ .../drm/amd/amdkfd/kfd_packet_manager_v9.c | 10 ++++++++ 4 files changed, 22 insertions(+), 18 deletions(-) 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 f157494bfdb1..4369308a74e7 100644 --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c @@ -1859,26 +1859,16 @@ static int start_cpsch(struct device_queue_manager *dqm) /* clear hang status when driver try to start the hw scheduler */ dqm->sched_running = true; - if (!dqm->dev->kfd->shared_resources.enable_mes) + if (!dqm->dev->kfd->shared_resources.enable_mes) { execute_queues_cpsch(dqm, KFD_UNMAP_QUEUES_FILTER_DYNAMIC_QUEUES, 0, USE_DEFAULT_GRACE_PERIOD); - - /* Set CWSR grace period to 1x1000 cycle for GFX9.4.3 APU */ - if (amdgpu_emu_mode == 0 && dqm->dev->adev->gmc.is_app_apu && - (KFD_GC_VERSION(dqm->dev) == IP_VERSION(9, 4, 3))) { - uint32_t reg_offset = 0; - uint32_t grace_period = 1; - - retval = pm_update_grace_period(&dqm->packet_mgr, - grace_period); + retval = pm_update_grace_period(&dqm->packet_mgr, SET_ASIC_OPTIMIZED_GRACE_PERIOD); if (retval) - dev_err(dev, "Setting grace timeout failed\n"); - else if (dqm->dev->kfd2kgd->build_grace_period_packet_info) - /* Update dqm->wait_times maintained in software */ - dqm->dev->kfd2kgd->build_grace_period_packet_info( - dqm->dev->adev, dqm->wait_times, - grace_period, &reg_offset, - &dqm->wait_times); + dev_err(dev, "Setting optimized grace timeout failed\n"); } + if (dqm->dev->kfd2kgd->get_iq_wait_times) + dqm->dev->kfd2kgd->get_iq_wait_times(dqm->dev->adev, + &dqm->wait_times, + ffs(dqm->dev->xcc_mask) - 1);</pre> </blockquote> why put it here? it seems not related to this patch. <blockquote type="cite" cite="mid:20250114195251.42661-1-Elena.Sakhnovitch@amd.com"> <pre wrap="" class="moz-quote-pre"> /* setup per-queue reset detection buffer */ num_hw_queue_slots = dqm->dev->kfd->shared_resources.num_queue_per_pipe * diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.h b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.h index 09ab36f8e8c6..fb3419993612 100644 --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.h +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.h @@ -37,7 +37,8 @@ #define KFD_MES_PROCESS_QUANTUM 100000 #define KFD_MES_GANG_QUANTUM 10000 -#define USE_DEFAULT_GRACE_PERIOD 0xffffffff +#define USE_DEFAULT_GRACE_PERIOD 0xffffffff</pre> </blockquote> what is difference between the two lines above, or just add spaces? <blockquote type="cite" cite="mid:20250114195251.42661-1-Elena.Sakhnovitch@amd.com"> <pre wrap="" class="moz-quote-pre"> +#define SET_ASIC_OPTIMIZED_GRACE_PERIOD 0xfffffffe struct device_process_node { struct qcm_process_device *qpd; diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager.c index 4984b41cd372..518c6ec23a75 100644 --- a/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager.c +++ b/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager.c @@ -403,6 +403,9 @@ int pm_update_grace_period(struct packet_manager *pm, uint32_t grace_period) int retval = 0; uint32_t *buffer, size; + if (!pm->pmf->set_grace_period || !pm->pmf->set_grace_period_size) + return 0; + size = pm->pmf->set_grace_period_size; mutex_lock(&pm->lock); diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_v9.c b/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_v9.c index d56525201155..fde212242129 100644 --- a/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_v9.c +++ b/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_v9.c @@ -302,9 +302,19 @@ static int pm_set_grace_period_v9(struct packet_manager *pm, uint32_t grace_period) { struct pm4_mec_write_data_mmio *packet; + struct device_queue_manager *dqm = pm->dqm; uint32_t reg_offset = 0; uint32_t reg_data = 0; + if (grace_period == SET_ASIC_OPTIMIZED_GRACE_PERIOD) {</pre> </blockquote> this patch purpose is setting grace_period at asic specific file. Do we need check "grace_period == SET_ASIC_OPTIMIZED_GRACE_PERIOD"? I think we can set grace_period directly at asic specific file. <blockquote type="cite" cite="mid:20250114195251.42661-1-Elena.Sakhnovitch@amd.com"> <pre wrap="" class="moz-quote-pre"> + /* Set CWSR grace period to 1x1000 cycle for GFX9.4.3 APU */ + if (amdgpu_emu_mode == 0 && dqm->dev->adev->gmc.is_app_apu && + KFD_GC_VERSION(dqm->dev) == IP_VERSION(9, 4, 3)) + grace_period = 1; + else + return 0;</pre> </blockquote> why return 0 here? that skip following build_grace_period_packet_info for no-GFX9.4.3 AP. As above, set grace_period directly is more straight forward at asic specific file. Regards Xiaogang <blockquote type="cite" cite="mid:20250114195251.42661-1-Elena.Sakhnovitch@amd.com"> <pre wrap="" class="moz-quote-pre"> + } + pm->dqm->dev->kfd2kgd->build_grace_period_packet_info( pm->dqm->dev->adev, pm->dqm->wait_times, </pre> </blockquote> </body> </html>