Re: [PATCH] drm/amdkfd: enable grace period for xcp instance

[Eric Huang <jinhuieric.huang@amd.com>]

On 2023-07-11 14:38, Felix Kuehling wrote:
>
> On 2023-07-11 10:28, Eric Huang wrote:
>> Read/write grace period from/to first xcc instance of
>> xcp in kfd node.
>>
>> Signed-off-by: Eric Huang <jinhuieric.huang@amd.com>
>> ---
>>   .../drm/amd/amdkfd/kfd_device_queue_manager.c | 21 ++++++++++++-------
>>   .../drm/amd/amdkfd/kfd_device_queue_manager.h |  2 +-
>>   .../drm/amd/amdkfd/kfd_packet_manager_v9.c    |  8 ++++---
>>   3 files changed, 20 insertions(+), 11 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 31cac1fd0d58..9000c4b778fd 100644
>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
>> @@ -1619,10 +1619,14 @@ static int initialize_cpsch(struct 
>> device_queue_manager *dqm)
>>         init_sdma_bitmaps(dqm);
>>   -    if (dqm->dev->kfd2kgd->get_iq_wait_times)
>> +    if (dqm->dev->kfd2kgd->get_iq_wait_times) {
>> +        u32 first_inst = dqm->dev->xcp->id *
>> +                 dqm->dev->adev->gfx.num_xcc_per_xcp;
>> dqm->dev->kfd2kgd->get_iq_wait_times(dqm->dev->adev,
>> -                    &dqm->wait_times,
>> -                    ffs(dqm->dev->xcc_mask) - 1);
>> +                    &dqm->wait_times[first_inst],
>> +                    first_inst);
>> +    }
>> +
>>       return 0;
>>   }
>>   @@ -1675,13 +1679,16 @@ static int start_cpsch(struct 
>> device_queue_manager *dqm)
>>                           grace_period);
>>           if (retval)
>>               pr_err("Setting grace timeout failed\n");
>> -        else if (dqm->dev->kfd2kgd->build_grace_period_packet_info)
>> +        else if (dqm->dev->kfd2kgd->build_grace_period_packet_info) {
>> +            u32 first_inst = dqm->dev->xcp->id *
>> + dqm->dev->adev->gfx.num_xcc_per_xcp;
>>               /* Update dqm->wait_times maintained in software */
>> dqm->dev->kfd2kgd->build_grace_period_packet_info(
>> -                    dqm->dev->adev, dqm->wait_times,
>> +                    dqm->dev->adev, dqm->wait_times[first_inst],
>>                       grace_period, &reg_offset,
>> -                    &dqm->wait_times,
>> -                    ffs(dqm->dev->xcc_mask) - 1);
>> +                    &dqm->wait_times[first_inst],
>> +                    first_inst);
>> +        }
>>       }
>>         dqm_unlock(dqm);
>> 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 7dd4b177219d..45959c33b944 100644
>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.h
>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.h
>> @@ -262,7 +262,7 @@ struct device_queue_manager {
>>       /* used for GFX 9.4.3 only */
>>       uint32_t        current_logical_xcc_start;
>>   -    uint32_t        wait_times;
>> +    uint32_t        wait_times[MAX_XCP];
>
> Why do you need an array here, if it only saves the wait times in one 
> of the array entries [first_inst]?
That is my misunderstanding for XCP. Each DPM should be associated to 1 
XCP. I thought DPM has multiple XCPs.

Thanks,
Eric
>
> Regards,
>   Felix
>
>
>>         wait_queue_head_t    destroy_wait;
>>   };
>> 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 8fda16e6fee6..960404a6379b 100644
>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_v9.c
>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_v9.c
>> @@ -292,17 +292,19 @@ static int pm_set_grace_period_v9(struct 
>> packet_manager *pm,
>>       struct pm4_mec_write_data_mmio *packet;
>>       uint32_t reg_offset = 0;
>>       uint32_t reg_data = 0;
>> +    uint32_t first_inst = pm->dqm->dev->xcp->id *
>> + pm->dqm->dev->adev->gfx.num_xcc_per_xcp;
>> pm->dqm->dev->kfd2kgd->build_grace_period_packet_info(
>>               pm->dqm->dev->adev,
>> -            pm->dqm->wait_times,
>> +            pm->dqm->wait_times[first_inst],
>>               grace_period,
>>               &reg_offset,
>>               &reg_data,
>> -            0);
>> +            first_inst);
>>         if (grace_period == USE_DEFAULT_GRACE_PERIOD)
>> -        reg_data = pm->dqm->wait_times;
>> +        reg_data = pm->dqm->wait_times[first_inst];
>>         packet = (struct pm4_mec_write_data_mmio *)buffer;
>>       memset(buffer, 0, sizeof(struct pm4_mec_write_data_mmio));