Re: [PATCH v15 4/6] drm/xe/pmu: Add attribute skeleton

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Wed, Jan 22, 2025 at 08:19:21PM -0800, Lucas De Marchi wrote:
> Add the generic support for defining new attributes. This uses
> gt-c6-residency as first attribute to bootstrap it, but its
> implementation will be added by a follow up commit: until proper support
> is added, it will always be invisible in sysfs since the corresponding
> bit is not set in the supported_events bitmap.

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> 
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_pmu.c       | 60 ++++++++++++++++++++++++++++---
>  drivers/gpu/drm/xe/xe_pmu_types.h |  4 +++
>  2 files changed, 60 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_pmu.c b/drivers/gpu/drm/xe/xe_pmu.c
> index 33598272db6aa..5e5a9fcf30ace 100644
> --- a/drivers/gpu/drm/xe/xe_pmu.c
> +++ b/drivers/gpu/drm/xe/xe_pmu.c
> @@ -46,6 +46,8 @@ static unsigned int config_to_gt_id(u64 config)
>  	return FIELD_GET(XE_PMU_EVENT_GT_MASK, config);
>  }
>  
> +#define XE_PMU_EVENT_GT_C6_RESIDENCY	0x01
> +
>  static struct xe_gt *event_to_gt(struct perf_event *event)
>  {
>  	struct xe_device *xe = container_of(event->pmu, typeof(*xe), pmu.base);
> @@ -60,7 +62,8 @@ static bool event_supported(struct xe_pmu *pmu, unsigned int gt,
>  	if (gt >= XE_MAX_GT_PER_TILE)
>  		return false;
>  
> -	return false;
> +	return id < sizeof(pmu->supported_events) * BITS_PER_BYTE &&
> +		pmu->supported_events & BIT_ULL(id);
>  }
>  
>  static void xe_pmu_event_destroy(struct perf_event *event)
> @@ -210,16 +213,62 @@ static const struct attribute_group pmu_format_attr_group = {
>  	.attrs = pmu_format_attrs,
>  };
>  
> -static struct attribute *pmu_event_attrs[] = {
> -	/* No events yet */
> +static ssize_t event_attr_show(struct device *dev,
> +			       struct device_attribute *attr, char *buf)
> +{
> +	struct perf_pmu_events_attr *pmu_attr =
> +		container_of(attr, struct perf_pmu_events_attr, attr);
> +
> +	return sprintf(buf, "event=%#04llx\n", pmu_attr->id);
> +}
> +
> +#define XE_EVENT_ATTR(name_, v_, id_, unit_)				\
> +	PMU_EVENT_ATTR(name_, pmu_event_ ## v_, id_, event_attr_show)	\
> +	PMU_EVENT_ATTR_STRING(name_.unit, pmu_event_unit_ ## v_, unit_)	\
> +	static struct attribute *pmu_attr_ ##v_[] = {			\
> +		&pmu_event_ ##v_.attr.attr,				\
> +		&pmu_event_unit_ ##v_.attr.attr,			\
> +		NULL							\
> +	};								\
> +	static umode_t is_visible_##v_(struct kobject *kobj,		\
> +				       struct attribute *attr, int idx) \
> +	{								\
> +		struct perf_pmu_events_attr *pmu_attr;			\
> +		struct xe_pmu *pmu;					\
> +									\
> +		pmu_attr = container_of(attr, typeof(*pmu_attr), attr.attr); \
> +		pmu = container_of(dev_get_drvdata(kobj_to_dev(kobj)),	\
> +				   typeof(*pmu), base);			\
> +									\
> +		return event_supported(pmu, 0, id_) ? attr->mode : 0;	\
> +	}								\
> +	static const struct attribute_group pmu_group_ ##v_ = {		\
> +		.name = "events",					\
> +		.attrs = pmu_attr_ ## v_,				\
> +		.is_visible = is_visible_ ## v_,			\
> +	}
> +
> +XE_EVENT_ATTR(gt-c6-residency, gt_c6_residency, XE_PMU_EVENT_GT_C6_RESIDENCY, "ms");
> +
> +static struct attribute *pmu_empty_event_attrs[] = {
> +	/* Empty - all events are added as groups with .attr_update() */
>  	NULL,
>  };
>  
>  static const struct attribute_group pmu_events_attr_group = {
>  	.name = "events",
> -	.attrs = pmu_event_attrs,
> +	.attrs = pmu_empty_event_attrs,
>  };
>  
> +static const struct attribute_group *pmu_events_attr_update[] = {
> +	&pmu_group_gt_c6_residency,
> +	NULL,
> +};
> +
> +static void set_supported_events(struct xe_pmu *pmu)
> +{
> +}
> +
>  /**
>   * xe_pmu_unregister() - Remove/cleanup PMU registration
>   * @arg: Ptr to pmu
> @@ -270,6 +319,7 @@ int xe_pmu_register(struct xe_pmu *pmu)
>  
>  	pmu->name		= name;
>  	pmu->base.attr_groups	= attr_groups;
> +	pmu->base.attr_update	= pmu_events_attr_update;
>  	pmu->base.scope		= PERF_PMU_SCOPE_SYS_WIDE;
>  	pmu->base.module	= THIS_MODULE;
>  	pmu->base.task_ctx_nr	= perf_invalid_context;
> @@ -280,6 +330,8 @@ int xe_pmu_register(struct xe_pmu *pmu)
>  	pmu->base.stop		= xe_pmu_event_stop;
>  	pmu->base.read		= xe_pmu_event_read;
>  
> +	set_supported_events(pmu);
> +
>  	ret = perf_pmu_register(&pmu->base, pmu->name, -1);
>  	if (ret)
>  		goto err_name;
> diff --git a/drivers/gpu/drm/xe/xe_pmu_types.h b/drivers/gpu/drm/xe/xe_pmu_types.h
> index 0e8faae6bc1b3..f5ba4d56622cb 100644
> --- a/drivers/gpu/drm/xe/xe_pmu_types.h
> +++ b/drivers/gpu/drm/xe/xe_pmu_types.h
> @@ -30,6 +30,10 @@ struct xe_pmu {
>  	 * @name: Name as registered with perf core.
>  	 */
>  	const char *name;
> +	/**
> +	 * @supported_events: Bitmap of supported events, indexed by event id
> +	 */
> +	u64 supported_events;
>  };
>  
>  #endif
> -- 
> 2.48.0
>