[PATCH 18/24] drm/amdkfd: enable pc sampling start

[James Zhu]

On 2023-11-22 17:27, Felix Kuehling wrote:
>
> On 2023-11-03 09:11, James Zhu wrote:
>> Enable pc sampling start.
>>
>> Signed-off-by: James Zhu <James.Zhu at amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdkfd/kfd_pc_sampling.c | 26 +++++++++++++++++---
>>   drivers/gpu/drm/amd/amdkfd/kfd_priv.h        |  2 ++
>>   2 files changed, 25 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_pc_sampling.c 
>> b/drivers/gpu/drm/amd/amdkfd/kfd_pc_sampling.c
>> index 60b29b245db5..33d003ca0093 100644
>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_pc_sampling.c
>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_pc_sampling.c
>> @@ -83,9 +83,29 @@ static int kfd_pc_sample_query_cap(struct 
>> kfd_process_device *pdd,
>>       return 0;
>>   }
>>   -static int kfd_pc_sample_start(struct kfd_process_device *pdd)
>> +static int kfd_pc_sample_start(struct kfd_process_device *pdd,
>> +                    struct pc_sampling_entry *pcs_entry)
>>   {
>> -    return -EINVAL;
>> +    bool pc_sampling_start = false;
>> +
>> +    pcs_entry->enabled = true;
>> +    mutex_lock(&pdd->dev->pcs_data.mutex);
>> +    if (!pdd->dev->pcs_data.hosttrap_entry.base.active_count)
>> +        pc_sampling_start = true;
>> + pdd->dev->pcs_data.hosttrap_entry.base.active_count++;
>> +    mutex_unlock(&pdd->dev->pcs_data.mutex);
>> +
>> +    while (pc_sampling_start) {
>> +        if 
>> (READ_ONCE(pdd->dev->pcs_data.hosttrap_entry.base.stop_enable)) {
>> +            usleep_range(1000, 2000);
>
> I don't understand why you need this synchronization through 
> stop_enable. Why can't you do both the start and stop while holding 
> the mutex? It's just setting a flag in the TMA, so it's not a 
> time-consuming operation, and I don't see any potential for deadlocks.
[JZ] for stop, not just set TMA. need wait for current pc sampling 
completely stop and reset some initial setting.
>
> Regards,
>   Felix
>
>
>> +        } else {
>> + kfd_process_set_trap_pc_sampling_flag(&pdd->qpd,
>> + pdd->dev->pcs_data.hosttrap_entry.base.pc_sample_info.method, true);
>> +            break;
>> +        }
>> +    }
>> +
>> +    return 0;
>>   }
>>     static int kfd_pc_sample_stop(struct kfd_process_device *pdd)
>> @@ -225,7 +245,7 @@ int kfd_pc_sample(struct kfd_process_device *pdd,
>>           if (pcs_entry->enabled)
>>               return -EALREADY;
>>           else
>> -            return kfd_pc_sample_start(pdd);
>> +            return kfd_pc_sample_start(pdd, pcs_entry);
>>         case KFD_IOCTL_PCS_OP_STOP:
>>           if (!pcs_entry->enabled)
>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h 
>> b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
>> index 6670534f47b8..613910e0d440 100644
>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
>> @@ -258,6 +258,8 @@ struct kfd_dev;
>>     struct kfd_dev_pc_sampling_data {
>>       uint32_t use_count;         /* Num of PC sampling sessions */
>> +    uint32_t active_count;      /* Num of active sessions */
>> +    bool stop_enable;           /* pc sampling stop in process */
>>       struct idr pc_sampling_idr;
>>       struct kfd_pc_sample_info pc_sample_info;
>>   };