Re: [Intel-xe] [PATCH 10/10] drm/xe/oa: Implement queries

[Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>]

On Mon, Aug 07, 2023 at 06:31:59PM -0700, Ashutosh Dixit wrote:
>Implement queries to query OA unit ID's for HW engines, OA timestamp freq
>and OA ioctl version.
>
>Signed-off-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
>---
> drivers/gpu/drm/xe/xe_oa.c    | 11 +++++++++++
> drivers/gpu/drm/xe/xe_oa.h    |  3 +++
> drivers/gpu/drm/xe/xe_query.c |  5 ++++-
> 3 files changed, 18 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/gpu/drm/xe/xe_oa.c b/drivers/gpu/drm/xe/xe_oa.c
>index fcbb352f36a48..f320e5cd76a13 100644
>--- a/drivers/gpu/drm/xe/xe_oa.c
>+++ b/drivers/gpu/drm/xe/xe_oa.c
>@@ -2219,6 +2219,12 @@ static int xe_oa_init_engine_groups(struct xe_oa *oa)
> 	return 0;
> }
>
>+u32 xe_oa_unit_id(struct xe_hw_engine *hwe)
>+{
>+	return hwe->oa_group && hwe->oa_group->num_engines ?
>+		hwe->oa_group->oa_unit_id : U32_MAX;
>+}
>+
> static void oa_format_add(struct xe_oa *oa, enum drm_xe_oa_format format)
> {
> 	__set_bit(format, oa->format_mask);
>@@ -2333,6 +2339,11 @@ static struct ctl_table oa_ctl_table[] = {
> 	{}
> };
>
>+int xe_oa_ioctl_version(struct xe_device *xe)
>+{
>+	return 1;
>+}
>+
> int xe_oa_sysctl_register(void)
> {
> 	sysctl_header = register_sysctl("dev/xe", oa_ctl_table);
>diff --git a/drivers/gpu/drm/xe/xe_oa.h b/drivers/gpu/drm/xe/xe_oa.h
>index fd6caf652047a..41a7d8b0f10e1 100644
>--- a/drivers/gpu/drm/xe/xe_oa.h
>+++ b/drivers/gpu/drm/xe/xe_oa.h
>@@ -22,4 +22,7 @@ int xe_oa_add_config_ioctl(struct drm_device *dev, void *data,
> 			   struct drm_file *file);
> int xe_oa_remove_config_ioctl(struct drm_device *dev, void *data,
> 			      struct drm_file *file);
>+u32 xe_oa_timestamp_frequency(struct xe_device *xe);
>+u32 xe_oa_unit_id(struct xe_hw_engine *hwe);
>+
> #endif
>diff --git a/drivers/gpu/drm/xe/xe_query.c b/drivers/gpu/drm/xe/xe_query.c
>index 7ea235c71385f..3dbc4bbae4d02 100644
>--- a/drivers/gpu/drm/xe/xe_query.c
>+++ b/drivers/gpu/drm/xe/xe_query.c
>@@ -78,7 +78,8 @@ static int query_engines(struct xe_device *xe,
> 				xe_to_user_engine_class[hwe->class];
> 			hw_engine_info[i].engine_instance =
> 				hwe->logical_instance;
>-			hw_engine_info[i++].gt_id = gt->info.id;
>+			hw_engine_info[i].gt_id = gt->info.id;
>+			hw_engine_info[i++].oa_unit_id = xe_oa_unit_id(hwe);

I would keep the i++ on a separate line for readability (even if the 
original code had it so).

Umesh
> 		}
>
> 	if (copy_to_user(query_ptr, hw_engine_info, size)) {
>@@ -204,6 +205,7 @@ static int query_config(struct xe_device *xe, struct drm_xe_device_query *query)
> 		hweight_long(xe->info.mem_region_mask);
> 	config->info[XE_QUERY_CONFIG_MAX_ENGINE_PRIORITY] =
> 		xe_exec_queue_device_get_max_priority(xe);
>+	config->info[XE_QUERY_OA_IOCTL_VERSION] = xe_oa_ioctl_version(xe);
>
> 	if (copy_to_user(query_ptr, config, size)) {
> 		kfree(config);
>@@ -245,6 +247,7 @@ static int query_gts(struct xe_device *xe, struct drm_xe_device_query *query)
> 			gts->gts[id].type = XE_QUERY_GT_TYPE_MAIN;
> 		gts->gts[id].instance = id;
> 		gts->gts[id].clock_freq = gt->info.clock_freq;
>+		gts->gts[id].oa_timestamp_freq = xe_oa_timestamp_frequency(xe);
> 		if (!IS_DGFX(xe))
> 			gts->gts[id].native_mem_regions = 0x1;
> 		else
>-- 
>2.41.0
>