[PATCH v2 03/23] drm/amdkfd: enable pc sampling query

[Yat Sin, David]

[AMD Official Use Only - General]



From: Zhu, James <James.Zhu at amd.com>
Sent: Wednesday, December 13, 2023 10:41 AM
To: Yat Sin, David <David.YatSin at amd.com>; Zhu, James <James.Zhu at amd.com>; amd-gfx at lists.freedesktop.org
Cc: Kuehling, Felix <Felix.Kuehling at amd.com>; Greathouse, Joseph <Joseph.Greathouse at amd.com>
Subject: Re: [PATCH v2 03/23] drm/amdkfd: enable pc sampling query



On 2023-12-12 19:55, Yat Sin, David wrote:

[AMD Official Use Only - General]



-----Original Message-----

From: Zhu, James <James.Zhu at amd.com><mailto:James.Zhu at amd.com>

Sent: Thursday, December 7, 2023 5:54 PM

To: amd-gfx at lists.freedesktop.org<mailto:amd-gfx at lists.freedesktop.org>

Cc: Kuehling, Felix <Felix.Kuehling at amd.com><mailto:Felix.Kuehling at amd.com>; Greathouse, Joseph

<Joseph.Greathouse at amd.com><mailto:Joseph.Greathouse at amd.com>; Yat Sin, David <David.YatSin at amd.com><mailto:David.YatSin at amd.com>;

Zhu, James <James.Zhu at amd.com><mailto:James.Zhu at amd.com>

Subject: [PATCH v2 03/23] drm/amdkfd: enable pc sampling query



From: David Yat Sin <david.yatsin at amd.com><mailto:david.yatsin at amd.com>



Enable pc sampling to query system capability.



Co-developed-by: James Zhu <James.Zhu at amd.com><mailto:James.Zhu at amd.com>

Signed-off-by: James Zhu <James.Zhu at amd.com><mailto:James.Zhu at amd.com>

Signed-off-by: David Yat Sin <david.yatsin at amd.com><mailto:david.yatsin at amd.com>

---

 drivers/gpu/drm/amd/amdkfd/kfd_pc_sampling.c | 54

+++++++++++++++++++-

 1 file changed, 53 insertions(+), 1 deletion(-)



diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_pc_sampling.c

b/drivers/gpu/drm/amd/amdkfd/kfd_pc_sampling.c

index a7e78ff42d07..49fecbc7013e 100644

--- a/drivers/gpu/drm/amd/amdkfd/kfd_pc_sampling.c

+++ b/drivers/gpu/drm/amd/amdkfd/kfd_pc_sampling.c

@@ -25,10 +25,62 @@

 #include "amdgpu_amdkfd.h"

 #include "kfd_pc_sampling.h"



+struct supported_pc_sample_info {

+     uint32_t ip_version;

+     const struct kfd_pc_sample_info *sample_info; };

+

+const struct kfd_pc_sample_info sample_info_hosttrap_9_0_0 = {

+     0, 1, ~0ULL, 0, KFD_IOCTL_PCS_METHOD_HOSTTRAP,

+KFD_IOCTL_PCS_TYPE_TIME_US };

+

+struct supported_pc_sample_info supported_formats[] = {

+     { IP_VERSION(9, 4, 1), &sample_info_hosttrap_9_0_0 },

+     { IP_VERSION(9, 4, 2), &sample_info_hosttrap_9_0_0 }, };

+

 static int kfd_pc_sample_query_cap(struct kfd_process_device *pdd,

                                      struct kfd_ioctl_pc_sample_args

__user *user_args)  {

-     return -EINVAL;

+     uint64_t sample_offset;

+     int num_method = 0;

+     int i;

+

+     for (i = 0; i < ARRAY_SIZE(supported_formats); i++)

+             if (KFD_GC_VERSION(pdd->dev) ==

supported_formats[i].ip_version)

+                     num_method++;

+

+     if (!num_method) {

+             pr_debug("PC Sampling not supported on GC_HWIP:0x%x.",

+                     pdd->dev->adev->ip_versions[GC_HWIP][0]);

+             return -EOPNOTSUPP;

+     }

+

+     if (!user_args->sample_info_ptr) {

Should be:

if (!user_args->sample_info_ptr || !user_args->num_sample_info) {



+             user_args->num_sample_info = num_method;

+             return 0;

+     }

+

+     if (user_args->num_sample_info < num_method) {

+             user_args->num_sample_info = num_method;

+             pr_debug("Sample info buffer is not large enough, "

+                      "ASIC requires space for %d kfd_pc_sample_info

entries.", num_method);

+             return -ENOSPC;

+     }

+

+     sample_offset = user_args->sample_info_ptr;



If there is another active PC Sampling session that is active, I thought we were planning to have code to

return a reduced list with only the methods that are compatible with the current active session. Did we

decide to drop this behavior?
[JZ] Do we have design changed here? I though we allow sharing the same active PC Sampling session between multiple processes.

[DavidYS] That was part of the original design.
For example, if you already have an active session that is taking samples at interval of 500ms, and a user does a capabilities query, then the
capabilities query should only return a range of 500ms, instead of the full interval range.





Regards,

David



+     for (i = 0; i < ARRAY_SIZE(supported_formats); i++) {

+             if (KFD_GC_VERSION(pdd->dev) ==

supported_formats[i].ip_version) {

+                     int ret = copy_to_user((void __user *) sample_offset,

+                             supported_formats[i].sample_info,

sizeof(struct kfd_pc_sample_info));

+                     if (ret) {

+                             pr_debug("Failed to copy PC sampling info to

user.");

+                             return -EFAULT;

+                     }

+                     sample_offset += sizeof(struct kfd_pc_sample_info);

+             }

+     }

+

+     return 0;

 }



 static int kfd_pc_sample_start(struct kfd_process_device *pdd)

--

2.25.1


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20231213/12f16ff0/attachment-0001.htm>