[PATCH 1/3] drm/amdgpu: fix xgmi perfmon a-b-a problem

[PATCH 1/3] drm/amdgpu: fix xgmi perfmon a-b-a problem

[Jonathan Kim]

Mapping hw counters per event config will cause ABA problems so map per
event instead.

v2: Discontinue starting perf counters if add fails.  Make it clear what's
happening with pmc_start.

Signed-off-by: Jonathan Kim <Jonathan.Kim@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_df.h  |   6 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c |  42 ++++++----
 drivers/gpu/drm/amd/amdgpu/df_v3_6.c    | 105 +++++++++++-------------
 3 files changed, 78 insertions(+), 75 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_df.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_df.h
index 373cdebe0e2f..52488bb45112 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_df.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_df.h
@@ -44,11 +44,11 @@ struct amdgpu_df_funcs {
 	void (*enable_ecc_force_par_wr_rmw)(struct amdgpu_device *adev,
 					    bool enable);
 	int (*pmc_start)(struct amdgpu_device *adev, uint64_t config,
-					 int is_add);
+					 int counter_idx, int is_add);
 	int (*pmc_stop)(struct amdgpu_device *adev, uint64_t config,
-					 int is_remove);
+					 int counter_idx, int is_remove);
 	void (*pmc_get_count)(struct amdgpu_device *adev, uint64_t config,
-					 uint64_t *count);
+					 int counter_idx, uint64_t *count);
 	uint64_t (*get_fica)(struct amdgpu_device *adev, uint32_t ficaa_val);
 	void (*set_fica)(struct amdgpu_device *adev, uint32_t ficaa_val,
 			 uint32_t ficadl_val, uint32_t ficadh_val);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
index 69af462db34d..1b0ec715c8ba 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
@@ -64,6 +64,7 @@ static void amdgpu_perf_start(struct perf_event *event, int flags)
 	struct amdgpu_pmu_entry *pe = container_of(event->pmu,
 						  struct amdgpu_pmu_entry,
 						  pmu);
+	int target_cntr = 0;
 
 	if (WARN_ON_ONCE(!(hwc->state & PERF_HES_STOPPED)))
 		return;
@@ -73,17 +74,24 @@ static void amdgpu_perf_start(struct perf_event *event, int flags)
 
 	switch (pe->pmu_perf_type) {
 	case PERF_TYPE_AMDGPU_DF:
-		if (!(flags & PERF_EF_RELOAD))
-			pe->adev->df.funcs->pmc_start(pe->adev, hwc->config, 1);
+		if (!(flags & PERF_EF_RELOAD)) {
+			target_cntr = pe->adev->df.funcs->pmc_start(pe->adev,
+						hwc->config, 0 /* unused */,
+						1 /* add counter */);
+			if (target_cntr < 0)
+				break;
+
+			hwc->idx = target_cntr;
+		}
 
-		pe->adev->df.funcs->pmc_start(pe->adev, hwc->config, 0);
+		pe->adev->df.funcs->pmc_start(pe->adev, hwc->config,
+								hwc->idx, 0);
 		break;
 	default:
 		break;
 	}
 
 	perf_event_update_userpage(event);
-
 }
 
 /* read perf counter */
@@ -101,8 +109,8 @@ static void amdgpu_perf_read(struct perf_event *event)
 
 		switch (pe->pmu_perf_type) {
 		case PERF_TYPE_AMDGPU_DF:
-			pe->adev->df.funcs->pmc_get_count(pe->adev, hwc->config,
-							  &count);
+			pe->adev->df.funcs->pmc_get_count(pe->adev,
+						hwc->config, hwc->idx, &count);
 			break;
 		default:
 			count = 0;
@@ -126,7 +134,8 @@ static void amdgpu_perf_stop(struct perf_event *event, int flags)
 
 	switch (pe->pmu_perf_type) {
 	case PERF_TYPE_AMDGPU_DF:
-		pe->adev->df.funcs->pmc_stop(pe->adev, hwc->config, 0);
+		pe->adev->df.funcs->pmc_stop(pe->adev, hwc->config, hwc->idx,
+									0);
 		break;
 	default:
 		break;
@@ -142,12 +151,11 @@ static void amdgpu_perf_stop(struct perf_event *event, int flags)
 	hwc->state |= PERF_HES_UPTODATE;
 }
 
-/* add perf counter  */
+/* add perf counter */
 static int amdgpu_perf_add(struct perf_event *event, int flags)
 {
 	struct hw_perf_event *hwc = &event->hw;
-	int retval;
-
+	int retval = 0, target_cntr;
 	struct amdgpu_pmu_entry *pe = container_of(event->pmu,
 						  struct amdgpu_pmu_entry,
 						  pmu);
@@ -156,8 +164,14 @@ static int amdgpu_perf_add(struct perf_event *event, int flags)
 
 	switch (pe->pmu_perf_type) {
 	case PERF_TYPE_AMDGPU_DF:
-		retval = pe->adev->df.funcs->pmc_start(pe->adev,
-						       hwc->config, 1);
+		target_cntr = pe->adev->df.funcs->pmc_start(pe->adev,
+						hwc->config, 0 /* unused */,
+						1 /* add counter */);
+		if (target_cntr < 0)
+			retval = target_cntr;
+		else
+			hwc->idx = target_cntr;
+
 		break;
 	default:
 		return 0;
@@ -170,7 +184,6 @@ static int amdgpu_perf_add(struct perf_event *event, int flags)
 		amdgpu_perf_start(event, PERF_EF_RELOAD);
 
 	return retval;
-
 }
 
 /* delete perf counter  */
@@ -185,7 +198,8 @@ static void amdgpu_perf_del(struct perf_event *event, int flags)
 
 	switch (pe->pmu_perf_type) {
 	case PERF_TYPE_AMDGPU_DF:
-		pe->adev->df.funcs->pmc_stop(pe->adev, hwc->config, 1);
+		pe->adev->df.funcs->pmc_stop(pe->adev, hwc->config, hwc->idx,
+									1);
 		break;
 	default:
 		break;
diff --git a/drivers/gpu/drm/amd/amdgpu/df_v3_6.c b/drivers/gpu/drm/amd/amdgpu/df_v3_6.c
index 7b89fd2aa44a..0ca6e176acb0 100644
--- a/drivers/gpu/drm/amd/amdgpu/df_v3_6.c
+++ b/drivers/gpu/drm/amd/amdgpu/df_v3_6.c
@@ -391,33 +391,28 @@ static void df_v3_6_get_clockgating_state(struct amdgpu_device *adev,
 }
 
 /* get assigned df perfmon ctr as int */
-static int df_v3_6_pmc_config_2_cntr(struct amdgpu_device *adev,
-				      uint64_t config)
+static bool df_v3_6_pmc_has_counter(struct amdgpu_device *adev,
+				      uint64_t config,
+				      int counter_idx)
 {
-	int i;
 
-	for (i = 0; i < DF_V3_6_MAX_COUNTERS; i++) {
-		if ((config & 0x0FFFFFFUL) ==
-					adev->df_perfmon_config_assign_mask[i])
-			return i;
-	}
+	return ((config & 0x0FFFFFFUL) ==
+			adev->df_perfmon_config_assign_mask[counter_idx]);
 
-	return -EINVAL;
 }
 
 /* get address based on counter assignment */
 static void df_v3_6_pmc_get_addr(struct amdgpu_device *adev,
 				 uint64_t config,
+				 int counter_idx,
 				 int is_ctrl,
 				 uint32_t *lo_base_addr,
 				 uint32_t *hi_base_addr)
 {
-	int target_cntr = df_v3_6_pmc_config_2_cntr(adev, config);
-
-	if (target_cntr < 0)
+	if (!df_v3_6_pmc_has_counter(adev, config, counter_idx))
 		return;
 
-	switch (target_cntr) {
+	switch (counter_idx) {
 
 	case 0:
 		*lo_base_addr = is_ctrl ? smnPerfMonCtlLo4 : smnPerfMonCtrLo4;
@@ -443,15 +438,18 @@ static void df_v3_6_pmc_get_addr(struct amdgpu_device *adev,
 /* get read counter address */
 static void df_v3_6_pmc_get_read_settings(struct amdgpu_device *adev,
 					  uint64_t config,
+					  int counter_idx,
 					  uint32_t *lo_base_addr,
 					  uint32_t *hi_base_addr)
 {
-	df_v3_6_pmc_get_addr(adev, config, 0, lo_base_addr, hi_base_addr);
+	df_v3_6_pmc_get_addr(adev, config, counter_idx, 0, lo_base_addr,
+								hi_base_addr);
 }
 
 /* get control counter settings i.e. address and values to set */
 static int df_v3_6_pmc_get_ctrl_settings(struct amdgpu_device *adev,
 					  uint64_t config,
+					  int counter_idx,
 					  uint32_t *lo_base_addr,
 					  uint32_t *hi_base_addr,
 					  uint32_t *lo_val,
@@ -462,7 +460,8 @@ static int df_v3_6_pmc_get_ctrl_settings(struct amdgpu_device *adev,
 	uint32_t eventsel, instance, unitmask;
 	uint32_t instance_10, instance_5432, instance_76;
 
-	df_v3_6_pmc_get_addr(adev, config, 1, lo_base_addr, hi_base_addr);
+	df_v3_6_pmc_get_addr(adev, config, counter_idx, 1, lo_base_addr,
+				hi_base_addr);
 
 	if ((*lo_base_addr == 0) || (*hi_base_addr == 0)) {
 		DRM_ERROR("[DF PMC] addressing not retrieved! Lo: %x, Hi: %x",
@@ -492,18 +491,13 @@ static int df_v3_6_pmc_get_ctrl_settings(struct amdgpu_device *adev,
 static int df_v3_6_pmc_add_cntr(struct amdgpu_device *adev,
 				   uint64_t config)
 {
-	int i, target_cntr;
-
-	target_cntr = df_v3_6_pmc_config_2_cntr(adev, config);
-
-	if (target_cntr >= 0)
-		return 0;
+	int i;
 
 	for (i = 0; i < DF_V3_6_MAX_COUNTERS; i++) {
 		if (adev->df_perfmon_config_assign_mask[i] == 0U) {
 			adev->df_perfmon_config_assign_mask[i] =
 							config & 0x0FFFFFFUL;
-			return 0;
+			return i;
 		}
 	}
 
@@ -512,59 +506,50 @@ static int df_v3_6_pmc_add_cntr(struct amdgpu_device *adev,
 
 #define DEFERRED_ARM_MASK	(1 << 31)
 static int df_v3_6_pmc_set_deferred(struct amdgpu_device *adev,
-				    uint64_t config, bool is_deferred)
+				    int counter_idx, uint64_t config,
+				    bool is_deferred)
 {
-	int target_cntr;
-
-	target_cntr = df_v3_6_pmc_config_2_cntr(adev, config);
 
-	if (target_cntr < 0)
+	if (!df_v3_6_pmc_has_counter(adev, config, counter_idx))
 		return -EINVAL;
 
 	if (is_deferred)
-		adev->df_perfmon_config_assign_mask[target_cntr] |=
+		adev->df_perfmon_config_assign_mask[counter_idx] |=
 							DEFERRED_ARM_MASK;
 	else
-		adev->df_perfmon_config_assign_mask[target_cntr] &=
+		adev->df_perfmon_config_assign_mask[counter_idx] &=
 							~DEFERRED_ARM_MASK;
 
 	return 0;
 }
 
 static bool df_v3_6_pmc_is_deferred(struct amdgpu_device *adev,
+				    int counter_idx,
 				    uint64_t config)
 {
-	int target_cntr;
-
-	target_cntr = df_v3_6_pmc_config_2_cntr(adev, config);
-
-	/*
-	 * we never get target_cntr < 0 since this funciton is only called in
-	 * pmc_count for now but we should check anyways.
-	 */
-	return (target_cntr >= 0 &&
-			(adev->df_perfmon_config_assign_mask[target_cntr]
-			& DEFERRED_ARM_MASK));
+	return	(df_v3_6_pmc_has_counter(adev, config, counter_idx) &&
+			(adev->df_perfmon_config_assign_mask[counter_idx]
+				& DEFERRED_ARM_MASK));
 
 }
 
 /* release performance counter */
 static void df_v3_6_pmc_release_cntr(struct amdgpu_device *adev,
-				     uint64_t config)
+				     uint64_t config,
+				     int counter_idx)
 {
-	int target_cntr = df_v3_6_pmc_config_2_cntr(adev, config);
-
-	if (target_cntr >= 0)
-		adev->df_perfmon_config_assign_mask[target_cntr] = 0ULL;
+	if (df_v3_6_pmc_has_counter(adev, config, counter_idx))
+		adev->df_perfmon_config_assign_mask[counter_idx] = 0ULL;
 }
 
 
 static void df_v3_6_reset_perfmon_cntr(struct amdgpu_device *adev,
-					 uint64_t config)
+					 uint64_t config,
+					 int counter_idx)
 {
 	uint32_t lo_base_addr = 0, hi_base_addr = 0;
 
-	df_v3_6_pmc_get_read_settings(adev, config, &lo_base_addr,
+	df_v3_6_pmc_get_read_settings(adev, config, counter_idx, &lo_base_addr,
 				      &hi_base_addr);
 
 	if ((lo_base_addr == 0) || (hi_base_addr == 0))
@@ -573,8 +558,9 @@ static void df_v3_6_reset_perfmon_cntr(struct amdgpu_device *adev,
 	df_v3_6_perfmon_wreg(adev, lo_base_addr, 0, hi_base_addr, 0);
 }
 
+/* return available counter if is_add == 1 otherwise return error status. */
 static int df_v3_6_pmc_start(struct amdgpu_device *adev, uint64_t config,
-			     int is_add)
+			     int counter_idx, int is_add)
 {
 	uint32_t lo_base_addr, hi_base_addr, lo_val, hi_val;
 	int err = 0, ret = 0;
@@ -584,10 +570,9 @@ static int df_v3_6_pmc_start(struct amdgpu_device *adev, uint64_t config,
 		if (is_add)
 			return df_v3_6_pmc_add_cntr(adev, config);
 
-		df_v3_6_reset_perfmon_cntr(adev, config);
-
 		ret = df_v3_6_pmc_get_ctrl_settings(adev,
 					config,
+					counter_idx,
 					&lo_base_addr,
 					&hi_base_addr,
 					&lo_val,
@@ -604,7 +589,8 @@ static int df_v3_6_pmc_start(struct amdgpu_device *adev, uint64_t config,
 						     hi_val);
 
 		if (err)
-			ret = df_v3_6_pmc_set_deferred(adev, config, true);
+			ret = df_v3_6_pmc_set_deferred(adev, config,
+							counter_idx, true);
 
 		break;
 	default:
@@ -615,7 +601,7 @@ static int df_v3_6_pmc_start(struct amdgpu_device *adev, uint64_t config,
 }
 
 static int df_v3_6_pmc_stop(struct amdgpu_device *adev, uint64_t config,
-			    int is_remove)
+			    int counter_idx, int is_remove)
 {
 	uint32_t lo_base_addr, hi_base_addr, lo_val, hi_val;
 	int ret = 0;
@@ -624,6 +610,7 @@ static int df_v3_6_pmc_stop(struct amdgpu_device *adev, uint64_t config,
 	case CHIP_VEGA20:
 		ret = df_v3_6_pmc_get_ctrl_settings(adev,
 			config,
+			counter_idx,
 			&lo_base_addr,
 			&hi_base_addr,
 			&lo_val,
@@ -635,8 +622,8 @@ static int df_v3_6_pmc_stop(struct amdgpu_device *adev, uint64_t config,
 
 
 		if (is_remove) {
-			df_v3_6_reset_perfmon_cntr(adev, config);
-			df_v3_6_pmc_release_cntr(adev, config);
+			df_v3_6_reset_perfmon_cntr(adev, config, counter_idx);
+			df_v3_6_pmc_release_cntr(adev, config, counter_idx);
 		}
 
 		break;
@@ -649,6 +636,7 @@ static int df_v3_6_pmc_stop(struct amdgpu_device *adev, uint64_t config,
 
 static void df_v3_6_pmc_get_count(struct amdgpu_device *adev,
 				  uint64_t config,
+				  int counter_idx,
 				  uint64_t *count)
 {
 	uint32_t lo_base_addr = 0, hi_base_addr = 0, lo_val = 0, hi_val = 0;
@@ -656,14 +644,14 @@ static void df_v3_6_pmc_get_count(struct amdgpu_device *adev,
 
 	switch (adev->asic_type) {
 	case CHIP_VEGA20:
-		df_v3_6_pmc_get_read_settings(adev, config, &lo_base_addr,
-				      &hi_base_addr);
+		df_v3_6_pmc_get_read_settings(adev, config, counter_idx,
+						&lo_base_addr, &hi_base_addr);
 
 		if ((lo_base_addr == 0) || (hi_base_addr == 0))
 			return;
 
 		/* rearm the counter or throw away count value on failure */
-		if (df_v3_6_pmc_is_deferred(adev, config)) {
+		if (df_v3_6_pmc_is_deferred(adev, config, counter_idx)) {
 			int rearm_err = df_v3_6_perfmon_arm_with_status(adev,
 							lo_base_addr, lo_val,
 							hi_base_addr, hi_val);
@@ -671,7 +659,8 @@ static void df_v3_6_pmc_get_count(struct amdgpu_device *adev,
 			if (rearm_err)
 				return;
 
-			df_v3_6_pmc_set_deferred(adev, config, false);
+			df_v3_6_pmc_set_deferred(adev, config, counter_idx,
+									false);
 		}
 
 		df_v3_6_perfmon_rreg(adev, lo_base_addr, &lo_val,
-- 
2.17.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 2/3] drm/amdgpu: add per device user friendly xgmi events for vega20

[Jonathan Kim]

Non-outbound data metrics are non useful so mark them as legacy.
Bucket new perf counters into device and not device ip.
Bind events to chip instead of IP.
Report available event counters and not number of hw counter banks.
Move DF public macros to private since not needed outside of IP version.

v2: add comments on sysfs structure and formatting.

Signed-off-by: Jonathan Kim <Jonathan.Kim@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h     |  13 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c | 342 +++++++++++++++++++++---
 drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.h |   6 +-
 drivers/gpu/drm/amd/amdgpu/df_v3_6.c    |  72 +----
 drivers/gpu/drm/amd/amdgpu/df_v3_6.h    |   9 -
 5 files changed, 314 insertions(+), 128 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 13f92dea182a..f43dfdd2716a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1279,19 +1279,6 @@ bool amdgpu_device_load_pci_state(struct pci_dev *pdev);
 
 #include "amdgpu_object.h"
 
-/* used by df_v3_6.c and amdgpu_pmu.c */
-#define AMDGPU_PMU_ATTR(_name, _object)					\
-static ssize_t								\
-_name##_show(struct device *dev,					\
-			       struct device_attribute *attr,		\
-			       char *page)				\
-{									\
-	BUILD_BUG_ON(sizeof(_object) >= PAGE_SIZE - 1);			\
-	return sprintf(page, _object "\n");				\
-}									\
-									\
-static struct device_attribute pmu_attr_##_name = __ATTR_RO(_name)
-
 static inline bool amdgpu_is_tmz(struct amdgpu_device *adev)
 {
        return adev->gmc.tmz_enabled;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
index 1b0ec715c8ba..f3d2ac0e88a7 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
@@ -27,9 +27,13 @@
 #include <linux/init.h>
 #include "amdgpu.h"
 #include "amdgpu_pmu.h"
-#include "df_v3_6.h"
 
 #define PMU_NAME_SIZE 32
+#define NUM_FORMATS_AMDGPU_PMU		4
+#define NUM_FORMATS_DF_LEGACY		3
+#define NUM_EVENTS_DF_LEGACY		8
+#define NUM_EVENTS_VEGA20_XGMI		2
+#define NUM_EVENTS_VEGA20_MAX		2
 
 /* record to keep track of pmu entry per pmu type per device */
 struct amdgpu_pmu_entry {
@@ -39,8 +43,106 @@ struct amdgpu_pmu_entry {
 	unsigned int pmu_perf_type;
 };
 
+struct amdgpu_pmu_event_attribute {
+	struct device_attribute attr;
+	const char *event_str;
+	unsigned int type;
+};
+
+static ssize_t amdgpu_pmu_event_show(struct device *dev,
+				struct device_attribute *attr, char *buf)
+{
+	struct amdgpu_pmu_event_attribute *amdgpu_pmu_attr;
+
+	amdgpu_pmu_attr = container_of(attr, struct amdgpu_pmu_event_attribute,
+									attr);
+
+	if (!amdgpu_pmu_attr->type)
+		return sprintf(buf, "%s\n", amdgpu_pmu_attr->event_str);
+
+	return sprintf(buf, "%s,type=0x%x\n",
+			amdgpu_pmu_attr->event_str, amdgpu_pmu_attr->type);
+}
+
+static struct attribute_group amdgpu_pmu_format_attr_group = {
+	.name = "format",
+	.attrs = NULL,
+};
+
+/*
+ * Event formatting is global to all amdgpu events under sysfs folder
+ * /sys/bus/event_source/devices/amdgpu_<dev_num> where dev_num is the
+ * primary device index. Registered events can be found in subfolder "events"
+ * and formatting under subfolder "format".
+ *
+ * Formats "event", "instance", and "umask" are currently used by xGMI but can
+ * be for generalized for other IP usage.  If format naming is insufficient
+ * for newly registered IP events, append to the list below and handle the
+ * perf events hardware configuration (see hwc->config) as required by the IP.
+ *
+ * Format "type" indicates IP type generated on pmu registration (see
+ * init_pmu_by_type) so non-legacy events omit this in the per-chip event
+ * list (e.g. vega20_events).
+ */
+static const char *amdgpu_pmu_formats[NUM_FORMATS_AMDGPU_PMU][2] = {
+	{ "event", "config:0-7" },
+	{ "instance", "config:8-15" },
+	{ "umask", "config:16-23"},
+	{ "type", "config:56-63"}
+};
+
 static LIST_HEAD(amdgpu_pmu_list);
 
+/* Vega20 events */
+static const char *vega20_events[NUM_EVENTS_VEGA20_MAX][2] = {
+	{ "xgmi_link0_data_outbound", "event=0x7,instance=0x46,umask=0x2" },
+	{ "xgmi_link1_data_outbound", "event=0x7,instance=0x47,umask=0x2" }
+};
+
+static struct attribute_group vega20_event_attr_group = {
+	.name = "events",
+	.attrs = NULL
+};
+
+const struct attribute_group *vega20_attr_groups[] = {
+	&amdgpu_pmu_format_attr_group,
+	&vega20_event_attr_group,
+	NULL
+};
+
+/* All df_vega20_* items are DEPRECATED. Use vega20_ items above instead. */
+static const char *df_vega20_formats[NUM_FORMATS_DF_LEGACY][2] = {
+	{ "event", "config:0-7" },
+	{ "instance", "config:8-15" },
+	{ "umask", "config:16-23"}
+};
+
+static const char *df_vega20_events[NUM_EVENTS_DF_LEGACY][2] = {
+	{ "cake0_pcsout_txdata", "event=0x7,instance=0x46,umask=0x2" },
+	{ "cake1_pcsout_txdata", "event=0x7,instance=0x47,umask=0x2" },
+	{ "cake0_pcsout_txmeta", "event=0x7,instance=0x46,umask=0x4" },
+	{ "cake1_pcsout_txmeta", "event=0x7,instance=0x47,umask=0x4" },
+	{ "cake0_ftiinstat_reqalloc", "event=0xb,instance=0x46,umask=0x4" },
+	{ "cake1_ftiinstat_reqalloc", "event=0xb,instance=0x47,umask=0x4" },
+	{ "cake0_ftiinstat_rspalloc", "event=0xb,instance=0x46,umask=0x8" },
+	{ "cake1_ftiinstat_rspalloc", "event=0xb,instance=0x47,umask=0x8" },
+};
+
+static struct attribute_group df_vega20_format_attr_group = {
+	.name = "format",
+	.attrs = NULL,
+};
+
+static struct attribute_group df_vega20_event_attr_group = {
+	.name = "events",
+	.attrs = NULL
+};
+
+const struct attribute_group *df_vega20_attr_groups[] = {
+	&df_vega20_format_attr_group,
+	&df_vega20_event_attr_group,
+	NULL
+};
 
 /* initialize perf counter */
 static int amdgpu_perf_event_init(struct perf_event *event)
@@ -73,7 +175,8 @@ static void amdgpu_perf_start(struct perf_event *event, int flags)
 	hwc->state = 0;
 
 	switch (pe->pmu_perf_type) {
-	case PERF_TYPE_AMDGPU_DF:
+	case PERF_TYPE_AMDGPU_DF_LEGACY:
+	case PERF_TYPE_AMDGPU_XGMI:
 		if (!(flags & PERF_EF_RELOAD)) {
 			target_cntr = pe->adev->df.funcs->pmc_start(pe->adev,
 						hwc->config, 0 /* unused */,
@@ -108,7 +211,8 @@ static void amdgpu_perf_read(struct perf_event *event)
 		prev = local64_read(&hwc->prev_count);
 
 		switch (pe->pmu_perf_type) {
-		case PERF_TYPE_AMDGPU_DF:
+		case PERF_TYPE_AMDGPU_DF_LEGACY:
+		case PERF_TYPE_AMDGPU_XGMI:
 			pe->adev->df.funcs->pmc_get_count(pe->adev,
 						hwc->config, hwc->idx, &count);
 			break;
@@ -133,7 +237,8 @@ static void amdgpu_perf_stop(struct perf_event *event, int flags)
 		return;
 
 	switch (pe->pmu_perf_type) {
-	case PERF_TYPE_AMDGPU_DF:
+	case PERF_TYPE_AMDGPU_DF_LEGACY:
+	case PERF_TYPE_AMDGPU_XGMI:
 		pe->adev->df.funcs->pmc_stop(pe->adev, hwc->config, hwc->idx,
 									0);
 		break;
@@ -160,10 +265,15 @@ static int amdgpu_perf_add(struct perf_event *event, int flags)
 						  struct amdgpu_pmu_entry,
 						  pmu);
 
+	if (pe->pmu_perf_type == PERF_TYPE_AMDGPU_MAX)
+		pe->pmu_perf_type = (hwc->config >> AMDGPU_PERF_TYPE_SHIFT) &
+							AMDGPU_PERF_TYPE_MASK;
+
 	event->hw.state = PERF_HES_UPTODATE | PERF_HES_STOPPED;
 
 	switch (pe->pmu_perf_type) {
-	case PERF_TYPE_AMDGPU_DF:
+	case PERF_TYPE_AMDGPU_DF_LEGACY:
+	case PERF_TYPE_AMDGPU_XGMI:
 		target_cntr = pe->adev->df.funcs->pmc_start(pe->adev,
 						hwc->config, 0 /* unused */,
 						1 /* add counter */);
@@ -197,7 +307,8 @@ static void amdgpu_perf_del(struct perf_event *event, int flags)
 	amdgpu_perf_stop(event, PERF_EF_UPDATE);
 
 	switch (pe->pmu_perf_type) {
-	case PERF_TYPE_AMDGPU_DF:
+	case PERF_TYPE_AMDGPU_DF_LEGACY:
+	case PERF_TYPE_AMDGPU_XGMI:
 		pe->adev->df.funcs->pmc_stop(pe->adev, hwc->config, hwc->idx,
 									1);
 		break;
@@ -208,18 +319,42 @@ static void amdgpu_perf_del(struct perf_event *event, int flags)
 	perf_event_update_userpage(event);
 }
 
-/* vega20 pmus */
+static void amdgpu_pmu_create_attributes(struct attribute_group *attr_group,
+				struct amdgpu_pmu_event_attribute *pmu_attr,
+				const char *events[][2],
+				int s_offset,
+				int e_offset,
+				unsigned int type)
+{
+	int i;
+
+	pmu_attr += s_offset;
+
+	for (i = s_offset; i < e_offset; i++) {
+		attr_group->attrs[i] = &pmu_attr->attr.attr;
+		sysfs_attr_init(&pmu_attr->attr.attr);
+		pmu_attr->attr.attr.name = events[i][0];
+		pmu_attr->attr.attr.mode = 0444;
+		pmu_attr->attr.show = amdgpu_pmu_event_show;
+		pmu_attr->event_str = events[i][1];
+		pmu_attr->type = type;
+		pmu_attr++;
+	}
+}
 
 /* init pmu tracking per pmu type */
 static int init_pmu_by_type(struct amdgpu_device *adev,
-		  const struct attribute_group *attr_groups[],
-		  char *pmu_type_name, char *pmu_file_prefix,
-		  unsigned int pmu_perf_type,
-		  unsigned int num_counters)
+			struct attribute_group *fmt_attr_group,
+			struct amdgpu_pmu_event_attribute *fmt_attr,
+			struct attribute_group *evt_attr_group,
+			struct amdgpu_pmu_event_attribute *evt_attr,
+			char *pmu_type_name, char *pmu_file_prefix,
+			unsigned int pmu_perf_type)
 {
 	char pmu_name[PMU_NAME_SIZE];
 	struct amdgpu_pmu_entry *pmu_entry;
-	int ret = 0;
+	bool is_legacy = pmu_perf_type == PERF_TYPE_AMDGPU_DF_LEGACY;
+	int ret = 0, num_events = 0;
 
 	pmu_entry = kzalloc(sizeof(struct amdgpu_pmu_entry), GFP_KERNEL);
 
@@ -237,59 +372,182 @@ static int init_pmu_by_type(struct amdgpu_device *adev,
 		.task_ctx_nr = perf_invalid_context,
 	};
 
-	pmu_entry->pmu.attr_groups = attr_groups;
+	amdgpu_pmu_create_attributes(fmt_attr_group, fmt_attr,
+			is_legacy ? df_vega20_formats : amdgpu_pmu_formats, 0,
+			is_legacy ? NUM_FORMATS_DF_LEGACY :
+							NUM_FORMATS_AMDGPU_PMU,
+			0);
+
+	if (is_legacy) {
+		amdgpu_pmu_create_attributes(evt_attr_group, evt_attr,
+					df_vega20_events, 0,
+					NUM_EVENTS_DF_LEGACY,
+					PERF_TYPE_AMDGPU_DF_LEGACY);
+		num_events += NUM_EVENTS_DF_LEGACY;
+
+		pmu_entry->pmu.attr_groups = df_vega20_attr_groups;
+		goto legacy_register;
+	}
+
+	switch (adev->asic_type) {
+	case CHIP_VEGA20:
+		amdgpu_pmu_create_attributes(evt_attr_group, evt_attr,
+				vega20_events, 0, NUM_EVENTS_VEGA20_XGMI,
+				PERF_TYPE_AMDGPU_XGMI);
+		num_events += NUM_EVENTS_VEGA20_XGMI;
+
+		/* other events can be added here */
+
+		pmu_entry->pmu.attr_groups = vega20_attr_groups;
+		break;
+	default:
+		return -ENODEV;
+	};
+
+legacy_register:
 	pmu_entry->pmu_perf_type = pmu_perf_type;
-	snprintf(pmu_name, PMU_NAME_SIZE, "%s_%d",
-				pmu_file_prefix, adev_to_drm(adev)->primary->index);
+	snprintf(pmu_name, PMU_NAME_SIZE, "%s_%d", pmu_file_prefix,
+					adev_to_drm(adev)->primary->index);
 
 	ret = perf_pmu_register(&pmu_entry->pmu, pmu_name, -1);
 
-	if (ret) {
-		kfree(pmu_entry);
-		pr_warn("Error initializing AMDGPU %s PMUs.\n", pmu_type_name);
-		return ret;
-	}
+	if (ret)
+		goto err;
 
 	pr_info("Detected AMDGPU %s Counters. # of Counters = %d.\n",
-			pmu_type_name, num_counters);
+			pmu_type_name, num_events);
 
 	list_add_tail(&pmu_entry->entry, &amdgpu_pmu_list);
 
 	return 0;
+err:
+	kfree(pmu_entry);
+	pr_warn("Error initializing AMDGPU %s PMUs.\n", pmu_type_name);
+	return ret;
+}
+
+/* destroy all pmu data associated with target device */
+void amdgpu_pmu_fini(struct amdgpu_device *adev)
+{
+	struct amdgpu_pmu_entry *pe, *temp;
+
+	list_for_each_entry_safe(pe, temp, &amdgpu_pmu_list, entry) {
+		if (pe->adev == adev) {
+			list_del(&pe->entry);
+			perf_pmu_unregister(&pe->pmu);
+			kfree(pe);
+		}
+	}
+}
+
+static int amdgpu_pmu_alloc_pmu_attrs(
+				struct attribute_group *fmt_attr_group,
+				struct amdgpu_pmu_event_attribute **fmt_attr,
+				int fmt_num_attrs,
+				struct attribute_group *evt_attr_group,
+				struct amdgpu_pmu_event_attribute **evt_attr,
+				int evt_num_attrs)
+{
+	*fmt_attr = kcalloc(fmt_num_attrs, sizeof(**fmt_attr), GFP_KERNEL);
+
+	if (!fmt_attr)
+		return -ENOMEM;
+
+	fmt_attr_group->attrs = kcalloc(fmt_num_attrs + 1,
+				sizeof(*fmt_attr_group->attrs), GFP_KERNEL);
+
+	if (!fmt_attr_group->attrs)
+		goto err_fmt_attr_grp;
+
+	*evt_attr = kcalloc(evt_num_attrs, sizeof(**evt_attr), GFP_KERNEL);
+
+	if (!evt_attr)
+		goto err_evt_attr;
+
+	evt_attr_group->attrs = kcalloc(evt_num_attrs + 1,
+				sizeof(*evt_attr_group->attrs), GFP_KERNEL);
+
+	if (!evt_attr_group->attrs)
+		goto err_evt_attr_grp;
+
+	return 0;
+err_evt_attr_grp:
+	kfree(*evt_attr);
+err_evt_attr:
+	kfree(fmt_attr_group->attrs);
+err_fmt_attr_grp:
+	kfree(*fmt_attr);
+	return -ENOMEM;
 }
 
 /* init amdgpu_pmu */
 int amdgpu_pmu_init(struct amdgpu_device *adev)
 {
+	struct amdgpu_pmu_event_attribute *fmt_attr = NULL, *evt_attr = NULL,
+				*old_fmt_attr = NULL, *old_evt_attr = NULL;
 	int ret = 0;
 
+	if (adev->asic_type != CHIP_VEGA20)
+		goto init_events;
+
+	ret = amdgpu_pmu_alloc_pmu_attrs(&df_vega20_format_attr_group,
+					&old_fmt_attr,
+					NUM_FORMATS_DF_LEGACY,
+					&df_vega20_event_attr_group,
+					&old_evt_attr,
+					NUM_EVENTS_DF_LEGACY);
+
+	if (ret)
+		return ret;
+
+	ret = init_pmu_by_type(adev,
+				&df_vega20_format_attr_group, old_fmt_attr,
+				&df_vega20_event_attr_group, old_evt_attr,
+				"DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF_LEGACY);
+	if (ret)
+		goto err_old_pmu;
+
+init_events:
 	switch (adev->asic_type) {
 	case CHIP_VEGA20:
-		/* init df */
-		ret = init_pmu_by_type(adev, df_v3_6_attr_groups,
-				       "DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF,
-				       DF_V3_6_MAX_COUNTERS);
+		ret = amdgpu_pmu_alloc_pmu_attrs(&amdgpu_pmu_format_attr_group,
+						&fmt_attr,
+						NUM_FORMATS_AMDGPU_PMU,
+						&vega20_event_attr_group,
+						&evt_attr,
+						NUM_EVENTS_VEGA20_MAX);
+
+		if (ret)
+			goto err_alloc;
+
+		ret = init_pmu_by_type(adev,
+				&amdgpu_pmu_format_attr_group, fmt_attr,
+				&vega20_event_attr_group, evt_attr,
+				"Event", "amdgpu", PERF_TYPE_AMDGPU_MAX);
+
+		if (ret) {
+			kfree(vega20_event_attr_group.attrs);
+			goto err_pmu;
+		}
 
-		/* other pmu types go here*/
 		break;
 	default:
 		return 0;
-	}
+	};
 
 	return 0;
-}
-
-
-/* destroy all pmu data associated with target device */
-void amdgpu_pmu_fini(struct amdgpu_device *adev)
-{
-	struct amdgpu_pmu_entry *pe, *temp;
-
-	list_for_each_entry_safe(pe, temp, &amdgpu_pmu_list, entry) {
-		if (pe->adev == adev) {
-			list_del(&pe->entry);
-			perf_pmu_unregister(&pe->pmu);
-			kfree(pe);
-		}
-	}
+err_pmu:
+	kfree(fmt_attr);
+	kfree(evt_attr);
+	kfree(amdgpu_pmu_format_attr_group.attrs);
+err_alloc:
+	if (adev->asic_type == CHIP_VEGA20)
+		amdgpu_pmu_fini(adev);
+	return ret;
+err_old_pmu:
+	kfree(old_fmt_attr);
+	kfree(old_evt_attr);
+	kfree(df_vega20_format_attr_group.attrs);
+	kfree(df_vega20_event_attr_group.attrs);
+	return ret;
 }
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.h
index 7dddb7160a11..0d214abe720e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.h
@@ -27,10 +27,14 @@
 #define _AMDGPU_PMU_H_
 
 enum amdgpu_pmu_perf_type {
-	PERF_TYPE_AMDGPU_DF = 0,
+	PERF_TYPE_AMDGPU_DF_LEGACY = 0,
+	PERF_TYPE_AMDGPU_XGMI,
 	PERF_TYPE_AMDGPU_MAX
 };
 
+#define AMDGPU_PERF_TYPE_SHIFT	56
+#define AMDGPU_PERF_TYPE_MASK	0xff
+
 int amdgpu_pmu_init(struct amdgpu_device *adev);
 void amdgpu_pmu_fini(struct amdgpu_device *adev);
 
diff --git a/drivers/gpu/drm/amd/amdgpu/df_v3_6.c b/drivers/gpu/drm/amd/amdgpu/df_v3_6.c
index 0ca6e176acb0..6e57ae95f997 100644
--- a/drivers/gpu/drm/amd/amdgpu/df_v3_6.c
+++ b/drivers/gpu/drm/amd/amdgpu/df_v3_6.c
@@ -30,71 +30,17 @@
 #define DF_3_6_SMN_REG_INST_DIST        0x8
 #define DF_3_6_INST_CNT                 8
 
-static u32 df_v3_6_channel_number[] = {1, 2, 0, 4, 0, 8, 0,
-				       16, 32, 0, 0, 0, 2, 4, 8};
-
-/* init df format attrs */
-AMDGPU_PMU_ATTR(event,		"config:0-7");
-AMDGPU_PMU_ATTR(instance,	"config:8-15");
-AMDGPU_PMU_ATTR(umask,		"config:16-23");
-
-/* df format attributes  */
-static struct attribute *df_v3_6_format_attrs[] = {
-	&pmu_attr_event.attr,
-	&pmu_attr_instance.attr,
-	&pmu_attr_umask.attr,
-	NULL
-};
-
-/* df format attribute group */
-static struct attribute_group df_v3_6_format_attr_group = {
-	.name = "format",
-	.attrs = df_v3_6_format_attrs,
-};
+/* Defined in global_features.h as FTI_PERFMON_VISIBLE */
+#define DF_V3_6_MAX_COUNTERS		4
 
-/* df event attrs */
-AMDGPU_PMU_ATTR(cake0_pcsout_txdata,
-		      "event=0x7,instance=0x46,umask=0x2");
-AMDGPU_PMU_ATTR(cake1_pcsout_txdata,
-		      "event=0x7,instance=0x47,umask=0x2");
-AMDGPU_PMU_ATTR(cake0_pcsout_txmeta,
-		      "event=0x7,instance=0x46,umask=0x4");
-AMDGPU_PMU_ATTR(cake1_pcsout_txmeta,
-		      "event=0x7,instance=0x47,umask=0x4");
-AMDGPU_PMU_ATTR(cake0_ftiinstat_reqalloc,
-		      "event=0xb,instance=0x46,umask=0x4");
-AMDGPU_PMU_ATTR(cake1_ftiinstat_reqalloc,
-		      "event=0xb,instance=0x47,umask=0x4");
-AMDGPU_PMU_ATTR(cake0_ftiinstat_rspalloc,
-		      "event=0xb,instance=0x46,umask=0x8");
-AMDGPU_PMU_ATTR(cake1_ftiinstat_rspalloc,
-		      "event=0xb,instance=0x47,umask=0x8");
-
-/* df event attributes  */
-static struct attribute *df_v3_6_event_attrs[] = {
-	&pmu_attr_cake0_pcsout_txdata.attr,
-	&pmu_attr_cake1_pcsout_txdata.attr,
-	&pmu_attr_cake0_pcsout_txmeta.attr,
-	&pmu_attr_cake1_pcsout_txmeta.attr,
-	&pmu_attr_cake0_ftiinstat_reqalloc.attr,
-	&pmu_attr_cake1_ftiinstat_reqalloc.attr,
-	&pmu_attr_cake0_ftiinstat_rspalloc.attr,
-	&pmu_attr_cake1_ftiinstat_rspalloc.attr,
-	NULL
-};
-
-/* df event attribute group */
-static struct attribute_group df_v3_6_event_attr_group = {
-	.name = "events",
-	.attrs = df_v3_6_event_attrs
-};
+/* get flags from df perfmon config */
+#define DF_V3_6_GET_EVENT(x)		(x & 0xFFUL)
+#define DF_V3_6_GET_INSTANCE(x)		((x >> 8) & 0xFFUL)
+#define DF_V3_6_GET_UNITMASK(x)		((x >> 16) & 0xFFUL)
+#define DF_V3_6_PERFMON_OVERFLOW	0xFFFFFFFFFFFFULL
 
-/* df event attr groups  */
-const struct attribute_group *df_v3_6_attr_groups[] = {
-		&df_v3_6_format_attr_group,
-		&df_v3_6_event_attr_group,
-		NULL
-};
+static u32 df_v3_6_channel_number[] = {1, 2, 0, 4, 0, 8, 0,
+				       16, 32, 0, 0, 0, 2, 4, 8};
 
 static uint64_t df_v3_6_get_fica(struct amdgpu_device *adev,
 				 uint32_t ficaa_val)
diff --git a/drivers/gpu/drm/amd/amdgpu/df_v3_6.h b/drivers/gpu/drm/amd/amdgpu/df_v3_6.h
index 76998541bc30..2505c7ef258a 100644
--- a/drivers/gpu/drm/amd/amdgpu/df_v3_6.h
+++ b/drivers/gpu/drm/amd/amdgpu/df_v3_6.h
@@ -35,15 +35,6 @@ enum DF_V3_6_MGCG {
 	DF_V3_6_MGCG_ENABLE_63_CYCLE_DELAY = 15
 };
 
-/* Defined in global_features.h as FTI_PERFMON_VISIBLE */
-#define DF_V3_6_MAX_COUNTERS		4
-
-/* get flags from df perfmon config */
-#define DF_V3_6_GET_EVENT(x)		(x & 0xFFUL)
-#define DF_V3_6_GET_INSTANCE(x)		((x >> 8) & 0xFFUL)
-#define DF_V3_6_GET_UNITMASK(x)		((x >> 16) & 0xFFUL)
-#define DF_V3_6_PERFMON_OVERFLOW	0xFFFFFFFFFFFFULL
-
 extern const struct attribute_group *df_v3_6_attr_groups[];
 extern const struct amdgpu_df_funcs df_v3_6_funcs;
 
-- 
2.17.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 3/3] drm/amdgpu: add xgmi perfmons for arcturus

[Jonathan Kim]

Add xgmi perfmons for Arcturus.

Signed-off-by: Jonathan Kim <Jonathan.Kim@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c | 55 +++++++++++++++++++++++++
 drivers/gpu/drm/amd/amdgpu/df_v3_6.c    |  3 ++
 2 files changed, 58 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
index f3d2ac0e88a7..ec521c72e631 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
@@ -34,6 +34,8 @@
 #define NUM_EVENTS_DF_LEGACY		8
 #define NUM_EVENTS_VEGA20_XGMI		2
 #define NUM_EVENTS_VEGA20_MAX		2
+#define NUM_EVENTS_ARCTURUS_XGMI	6
+#define NUM_EVENTS_ARCTURUS_MAX		6
 
 /* record to keep track of pmu entry per pmu type per device */
 struct amdgpu_pmu_entry {
@@ -110,6 +112,27 @@ const struct attribute_group *vega20_attr_groups[] = {
 	NULL
 };
 
+/* Arcturus events */
+static const char *arcturus_events[NUM_EVENTS_ARCTURUS_MAX][2] = {
+	{ "xgmi_link0_data_outbound", "event=0x7,instance=0x4b,umask=0x2" },
+	{ "xgmi_link1_data_outbound", "event=0x7,instance=0x4c,umask=0x2" },
+	{ "xgmi_link2_data_outbound", "event=0x7,instance=0x4d,umask=0x2" },
+	{ "xgmi_link3_data_outbound", "event=0x7,instance=0x4e,umask=0x2" },
+	{ "xgmi_link4_data_outbound", "event=0x7,instance=0x4f,umask=0x2" },
+	{ "xgmi_link5_data_outbound", "event=0x7,instance=0x50,umask=0x2" }
+};
+
+static struct attribute_group arcturus_event_attr_group = {
+	.name = "events",
+	.attrs = NULL
+};
+
+const struct attribute_group *arcturus_attr_groups[] = {
+	&amdgpu_pmu_format_attr_group,
+	&arcturus_event_attr_group,
+	NULL
+};
+
 /* All df_vega20_* items are DEPRECATED. Use vega20_ items above instead. */
 static const char *df_vega20_formats[NUM_FORMATS_DF_LEGACY][2] = {
 	{ "event", "config:0-7" },
@@ -400,6 +423,16 @@ static int init_pmu_by_type(struct amdgpu_device *adev,
 
 		pmu_entry->pmu.attr_groups = vega20_attr_groups;
 		break;
+	case CHIP_ARCTURUS:
+		amdgpu_pmu_create_attributes(evt_attr_group, evt_attr,
+				arcturus_events, 0, NUM_EVENTS_ARCTURUS_XGMI,
+				PERF_TYPE_AMDGPU_XGMI);
+		num_events += NUM_EVENTS_ARCTURUS_XGMI;
+
+		/* other events can be added here */
+
+		pmu_entry->pmu.attr_groups = arcturus_attr_groups;
+		break;
 	default:
 		return -ENODEV;
 	};
@@ -530,6 +563,28 @@ int amdgpu_pmu_init(struct amdgpu_device *adev)
 			goto err_pmu;
 		}
 
+		break;
+	case CHIP_ARCTURUS:
+		ret = amdgpu_pmu_alloc_pmu_attrs(&amdgpu_pmu_format_attr_group,
+						&fmt_attr,
+						NUM_FORMATS_AMDGPU_PMU,
+						&arcturus_event_attr_group,
+						&evt_attr,
+						NUM_EVENTS_ARCTURUS_MAX);
+
+		if (ret)
+			goto err_alloc;
+
+		ret = init_pmu_by_type(adev,
+				&amdgpu_pmu_format_attr_group, fmt_attr,
+				&arcturus_event_attr_group, evt_attr,
+				"Event", "amdgpu", PERF_TYPE_AMDGPU_MAX);
+
+		if (ret) {
+			kfree(arcturus_event_attr_group.attrs);
+			goto err_pmu;
+		}
+
 		break;
 	default:
 		return 0;
diff --git a/drivers/gpu/drm/amd/amdgpu/df_v3_6.c b/drivers/gpu/drm/amd/amdgpu/df_v3_6.c
index 6e57ae95f997..6b4b30a8dce5 100644
--- a/drivers/gpu/drm/amd/amdgpu/df_v3_6.c
+++ b/drivers/gpu/drm/amd/amdgpu/df_v3_6.c
@@ -513,6 +513,7 @@ static int df_v3_6_pmc_start(struct amdgpu_device *adev, uint64_t config,
 
 	switch (adev->asic_type) {
 	case CHIP_VEGA20:
+	case CHIP_ARCTURUS:
 		if (is_add)
 			return df_v3_6_pmc_add_cntr(adev, config);
 
@@ -554,6 +555,7 @@ static int df_v3_6_pmc_stop(struct amdgpu_device *adev, uint64_t config,
 
 	switch (adev->asic_type) {
 	case CHIP_VEGA20:
+	case CHIP_ARCTURUS:
 		ret = df_v3_6_pmc_get_ctrl_settings(adev,
 			config,
 			counter_idx,
@@ -590,6 +592,7 @@ static void df_v3_6_pmc_get_count(struct amdgpu_device *adev,
 
 	switch (adev->asic_type) {
 	case CHIP_VEGA20:
+	case CHIP_ARCTURUS:
 		df_v3_6_pmc_get_read_settings(adev, config, counter_idx,
 						&lo_base_addr, &hi_base_addr);
 
-- 
2.17.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

RE: [PATCH 2/3] drm/amdgpu: add per device user friendly xgmi events for vega20

[Kasiviswanathan, Harish]

[AMD Official Use Only - Internal Distribution Only]

Some comments and a question

-----Original Message-----
From: Kim, Jonathan <Jonathan.Kim@amd.com> 
Sent: Tuesday, September 15, 2020 6:00 PM
To: amd-gfx@lists.freedesktop.org
Cc: Kasiviswanathan, Harish <Harish.Kasiviswanathan@amd.com>; Kim, Jonathan <Jonathan.Kim@amd.com>; Kim, Jonathan <Jonathan.Kim@amd.com>
Subject: [PATCH 2/3] drm/amdgpu: add per device user friendly xgmi events for vega20

Non-outbound data metrics are non useful so mark them as legacy.
Bucket new perf counters into device and not device ip.
Bind events to chip instead of IP.
Report available event counters and not number of hw counter banks.
Move DF public macros to private since not needed outside of IP version.

v2: add comments on sysfs structure and formatting.

Signed-off-by: Jonathan Kim <Jonathan.Kim@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h     |  13 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c | 342 +++++++++++++++++++++---
 drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.h |   6 +-
 drivers/gpu/drm/amd/amdgpu/df_v3_6.c    |  72 +----
 drivers/gpu/drm/amd/amdgpu/df_v3_6.h    |   9 -
 5 files changed, 314 insertions(+), 128 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 13f92dea182a..f43dfdd2716a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1279,19 +1279,6 @@ bool amdgpu_device_load_pci_state(struct pci_dev *pdev);
 
 #include "amdgpu_object.h"
 
-/* used by df_v3_6.c and amdgpu_pmu.c */
-#define AMDGPU_PMU_ATTR(_name, _object)					\
-static ssize_t								\
-_name##_show(struct device *dev,					\
-			       struct device_attribute *attr,		\
-			       char *page)				\
-{									\
-	BUILD_BUG_ON(sizeof(_object) >= PAGE_SIZE - 1);			\
-	return sprintf(page, _object "\n");				\
-}									\
-									\
-static struct device_attribute pmu_attr_##_name = __ATTR_RO(_name)
-
 static inline bool amdgpu_is_tmz(struct amdgpu_device *adev)
 {
        return adev->gmc.tmz_enabled;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
index 1b0ec715c8ba..f3d2ac0e88a7 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
@@ -27,9 +27,13 @@
 #include <linux/init.h>
 #include "amdgpu.h"
 #include "amdgpu_pmu.h"
-#include "df_v3_6.h"
 
 #define PMU_NAME_SIZE 32
+#define NUM_FORMATS_AMDGPU_PMU		4
+#define NUM_FORMATS_DF_LEGACY		3
+#define NUM_EVENTS_DF_LEGACY		8
+#define NUM_EVENTS_VEGA20_XGMI		2
+#define NUM_EVENTS_VEGA20_MAX		2
 
 /* record to keep track of pmu entry per pmu type per device */
 struct amdgpu_pmu_entry {
@@ -39,8 +43,106 @@ struct amdgpu_pmu_entry {
 	unsigned int pmu_perf_type;
 };
 
+struct amdgpu_pmu_event_attribute {
+	struct device_attribute attr;
+	const char *event_str;
+	unsigned int type;
+};
+
+static ssize_t amdgpu_pmu_event_show(struct device *dev,
+				struct device_attribute *attr, char *buf)
+{
+	struct amdgpu_pmu_event_attribute *amdgpu_pmu_attr;
+
+	amdgpu_pmu_attr = container_of(attr, struct amdgpu_pmu_event_attribute,
+									attr);
+
+	if (!amdgpu_pmu_attr->type)
+		return sprintf(buf, "%s\n", amdgpu_pmu_attr->event_str);
+
+	return sprintf(buf, "%s,type=0x%x\n",
+			amdgpu_pmu_attr->event_str, amdgpu_pmu_attr->type);
+}
+
+static struct attribute_group amdgpu_pmu_format_attr_group = {
+	.name = "format",
+	.attrs = NULL,
+};
+
+/*
+ * Event formatting is global to all amdgpu events under sysfs folder
+ * /sys/bus/event_source/devices/amdgpu_<dev_num> where dev_num is the
+ * primary device index. Registered events can be found in subfolder "events"
+ * and formatting under subfolder "format".
+ *
+ * Formats "event", "instance", and "umask" are currently used by xGMI but can
+ * be for generalized for other IP usage.  If format naming is insufficient
+ * for newly registered IP events, append to the list below and handle the
+ * perf events hardware configuration (see hwc->config) as required by the IP.
+ *
+ * Format "type" indicates IP type generated on pmu registration (see
+ * init_pmu_by_type) so non-legacy events omit this in the per-chip event
+ * list (e.g. vega20_events).
+ */
+static const char *amdgpu_pmu_formats[NUM_FORMATS_AMDGPU_PMU][2] = {
+	{ "event", "config:0-7" },
+	{ "instance", "config:8-15" },
+	{ "umask", "config:16-23"},
+	{ "type", "config:56-63"}
+};
+
 static LIST_HEAD(amdgpu_pmu_list);
 
+/* Vega20 events */
+static const char *vega20_events[NUM_EVENTS_VEGA20_MAX][2] = {
+	{ "xgmi_link0_data_outbound", "event=0x7,instance=0x46,umask=0x2" },
+	{ "xgmi_link1_data_outbound", "event=0x7,instance=0x47,umask=0x2" }
+};
+
+static struct attribute_group vega20_event_attr_group = {
+	.name = "events",
+	.attrs = NULL
+};
+
+const struct attribute_group *vega20_attr_groups[] = {
+	&amdgpu_pmu_format_attr_group,
+	&vega20_event_attr_group,
+	NULL
+};
+
+/* All df_vega20_* items are DEPRECATED. Use vega20_ items above instead. */
+static const char *df_vega20_formats[NUM_FORMATS_DF_LEGACY][2] = {
+	{ "event", "config:0-7" },
+	{ "instance", "config:8-15" },
+	{ "umask", "config:16-23"}
+};
+
+static const char *df_vega20_events[NUM_EVENTS_DF_LEGACY][2] = {
+	{ "cake0_pcsout_txdata", "event=0x7,instance=0x46,umask=0x2" },
+	{ "cake1_pcsout_txdata", "event=0x7,instance=0x47,umask=0x2" },
+	{ "cake0_pcsout_txmeta", "event=0x7,instance=0x46,umask=0x4" },
+	{ "cake1_pcsout_txmeta", "event=0x7,instance=0x47,umask=0x4" },
+	{ "cake0_ftiinstat_reqalloc", "event=0xb,instance=0x46,umask=0x4" },
+	{ "cake1_ftiinstat_reqalloc", "event=0xb,instance=0x47,umask=0x4" },
+	{ "cake0_ftiinstat_rspalloc", "event=0xb,instance=0x46,umask=0x8" },
+	{ "cake1_ftiinstat_rspalloc", "event=0xb,instance=0x47,umask=0x8" },
+};
+
+static struct attribute_group df_vega20_format_attr_group = {
+	.name = "format",
+	.attrs = NULL,
+};
+
+static struct attribute_group df_vega20_event_attr_group = {
+	.name = "events",
+	.attrs = NULL
+};
+
+const struct attribute_group *df_vega20_attr_groups[] = {
+	&df_vega20_format_attr_group,
+	&df_vega20_event_attr_group,
+	NULL
+};
 
 /* initialize perf counter */
 static int amdgpu_perf_event_init(struct perf_event *event)
@@ -73,7 +175,8 @@ static void amdgpu_perf_start(struct perf_event *event, int flags)
 	hwc->state = 0;
 
 	switch (pe->pmu_perf_type) {
-	case PERF_TYPE_AMDGPU_DF:
+	case PERF_TYPE_AMDGPU_DF_LEGACY:
+	case PERF_TYPE_AMDGPU_XGMI:
 		if (!(flags & PERF_EF_RELOAD)) {
 			target_cntr = pe->adev->df.funcs->pmc_start(pe->adev,
 						hwc->config, 0 /* unused */,
@@ -108,7 +211,8 @@ static void amdgpu_perf_read(struct perf_event *event)
 		prev = local64_read(&hwc->prev_count);
 
 		switch (pe->pmu_perf_type) {
-		case PERF_TYPE_AMDGPU_DF:
+		case PERF_TYPE_AMDGPU_DF_LEGACY:
+		case PERF_TYPE_AMDGPU_XGMI:
 			pe->adev->df.funcs->pmc_get_count(pe->adev,
 						hwc->config, hwc->idx, &count);
 			break;
@@ -133,7 +237,8 @@ static void amdgpu_perf_stop(struct perf_event *event, int flags)
 		return;
 
 	switch (pe->pmu_perf_type) {
-	case PERF_TYPE_AMDGPU_DF:
+	case PERF_TYPE_AMDGPU_DF_LEGACY:
+	case PERF_TYPE_AMDGPU_XGMI:
 		pe->adev->df.funcs->pmc_stop(pe->adev, hwc->config, hwc->idx,
 									0);
 		break;
@@ -160,10 +265,15 @@ static int amdgpu_perf_add(struct perf_event *event, int flags)
 						  struct amdgpu_pmu_entry,
 						  pmu);
 
+	if (pe->pmu_perf_type == PERF_TYPE_AMDGPU_MAX)
+		pe->pmu_perf_type = (hwc->config >> AMDGPU_PERF_TYPE_SHIFT) &
+							AMDGPU_PERF_TYPE_MASK;
+
 	event->hw.state = PERF_HES_UPTODATE | PERF_HES_STOPPED;
 
 	switch (pe->pmu_perf_type) {
-	case PERF_TYPE_AMDGPU_DF:
+	case PERF_TYPE_AMDGPU_DF_LEGACY:
+	case PERF_TYPE_AMDGPU_XGMI:
 		target_cntr = pe->adev->df.funcs->pmc_start(pe->adev,
 						hwc->config, 0 /* unused */,
 						1 /* add counter */);
@@ -197,7 +307,8 @@ static void amdgpu_perf_del(struct perf_event *event, int flags)
 	amdgpu_perf_stop(event, PERF_EF_UPDATE);
 
 	switch (pe->pmu_perf_type) {
-	case PERF_TYPE_AMDGPU_DF:
+	case PERF_TYPE_AMDGPU_DF_LEGACY:
+	case PERF_TYPE_AMDGPU_XGMI:
 		pe->adev->df.funcs->pmc_stop(pe->adev, hwc->config, hwc->idx,
 									1);
 		break;
@@ -208,18 +319,42 @@ static void amdgpu_perf_del(struct perf_event *event, int flags)
 	perf_event_update_userpage(event);
 }
 
-/* vega20 pmus */
+static void amdgpu_pmu_create_attributes(struct attribute_group *attr_group,
+				struct amdgpu_pmu_event_attribute *pmu_attr,
+				const char *events[][2],
+				int s_offset,
+				int e_offset,
+				unsigned int type)
+{
+	int i;
+
+	pmu_attr += s_offset;
+
+	for (i = s_offset; i < e_offset; i++) {
+		attr_group->attrs[i] = &pmu_attr->attr.attr;
+		sysfs_attr_init(&pmu_attr->attr.attr);
+		pmu_attr->attr.attr.name = events[i][0];
+		pmu_attr->attr.attr.mode = 0444;
+		pmu_attr->attr.show = amdgpu_pmu_event_show;
+		pmu_attr->event_str = events[i][1];
+		pmu_attr->type = type;
+		pmu_attr++;
+	}
+}
 
 /* init pmu tracking per pmu type */
 static int init_pmu_by_type(struct amdgpu_device *adev,
-		  const struct attribute_group *attr_groups[],
-		  char *pmu_type_name, char *pmu_file_prefix,
-		  unsigned int pmu_perf_type,
-		  unsigned int num_counters)
+			struct attribute_group *fmt_attr_group,
+			struct amdgpu_pmu_event_attribute *fmt_attr,
+			struct attribute_group *evt_attr_group,
+			struct amdgpu_pmu_event_attribute *evt_attr,
+			char *pmu_type_name, char *pmu_file_prefix,
+			unsigned int pmu_perf_type)
 {
 	char pmu_name[PMU_NAME_SIZE];
 	struct amdgpu_pmu_entry *pmu_entry;
-	int ret = 0;
+	bool is_legacy = pmu_perf_type == PERF_TYPE_AMDGPU_DF_LEGACY;
+	int ret = 0, num_events = 0;
 
 	pmu_entry = kzalloc(sizeof(struct amdgpu_pmu_entry), GFP_KERNEL);
 
@@ -237,59 +372,182 @@ static int init_pmu_by_type(struct amdgpu_device *adev,
 		.task_ctx_nr = perf_invalid_context,
 	};
 
-	pmu_entry->pmu.attr_groups = attr_groups;
+	amdgpu_pmu_create_attributes(fmt_attr_group, fmt_attr,
+			is_legacy ? df_vega20_formats : amdgpu_pmu_formats, 0,
+			is_legacy ? NUM_FORMATS_DF_LEGACY :
+							NUM_FORMATS_AMDGPU_PMU,
+			0);
+
+	if (is_legacy) {
+		amdgpu_pmu_create_attributes(evt_attr_group, evt_attr,
+					df_vega20_events, 0,
+					NUM_EVENTS_DF_LEGACY,
+					PERF_TYPE_AMDGPU_DF_LEGACY);
+		num_events += NUM_EVENTS_DF_LEGACY;
+
+		pmu_entry->pmu.attr_groups = df_vega20_attr_groups;
+		goto legacy_register;
+	}
+
+	switch (adev->asic_type) {
+	case CHIP_VEGA20:
+		amdgpu_pmu_create_attributes(evt_attr_group, evt_attr,
+				vega20_events, 0, NUM_EVENTS_VEGA20_XGMI,
+				PERF_TYPE_AMDGPU_XGMI);
+		num_events += NUM_EVENTS_VEGA20_XGMI;
+
+		/* other events can be added here */
+
+		pmu_entry->pmu.attr_groups = vega20_attr_groups;
+		break;
+	default:
+		return -ENODEV;
+	};
+
+legacy_register:
 	pmu_entry->pmu_perf_type = pmu_perf_type;
-	snprintf(pmu_name, PMU_NAME_SIZE, "%s_%d",
-				pmu_file_prefix, adev_to_drm(adev)->primary->index);
+	snprintf(pmu_name, PMU_NAME_SIZE, "%s_%d", pmu_file_prefix,
+					adev_to_drm(adev)->primary->index);
 
 	ret = perf_pmu_register(&pmu_entry->pmu, pmu_name, -1);
 
-	if (ret) {
-		kfree(pmu_entry);
-		pr_warn("Error initializing AMDGPU %s PMUs.\n", pmu_type_name);
-		return ret;
-	}
+	if (ret)
+		goto err;
 
 	pr_info("Detected AMDGPU %s Counters. # of Counters = %d.\n",
-			pmu_type_name, num_counters);
+			pmu_type_name, num_events);
 
 	list_add_tail(&pmu_entry->entry, &amdgpu_pmu_list);
 
 	return 0;
+err:
+	kfree(pmu_entry);
+	pr_warn("Error initializing AMDGPU %s PMUs.\n", pmu_type_name);
+	return ret;
+}
+
+/* destroy all pmu data associated with target device */
+void amdgpu_pmu_fini(struct amdgpu_device *adev)
+{
+	struct amdgpu_pmu_entry *pe, *temp;
+
+	list_for_each_entry_safe(pe, temp, &amdgpu_pmu_list, entry) {
+		if (pe->adev == adev) {
+			list_del(&pe->entry);
+			perf_pmu_unregister(&pe->pmu);
+			kfree(pe);
+		}
+	}
+}
+
+static int amdgpu_pmu_alloc_pmu_attrs(
+				struct attribute_group *fmt_attr_group,
+				struct amdgpu_pmu_event_attribute **fmt_attr,
+				int fmt_num_attrs,
+				struct attribute_group *evt_attr_group,
+				struct amdgpu_pmu_event_attribute **evt_attr,
+				int evt_num_attrs)
+{

where do all these allocations get freed?

+	*fmt_attr = kcalloc(fmt_num_attrs, sizeof(**fmt_attr), GFP_KERNEL);
+
+	if (!fmt_attr)


I think you want to check *fmt_attr

+		return -ENOMEM;
+
+	fmt_attr_group->attrs = kcalloc(fmt_num_attrs + 1,
+				sizeof(*fmt_attr_group->attrs), GFP_KERNEL);
+
+	if (!fmt_attr_group->attrs)
+		goto err_fmt_attr_grp;
+
+	*evt_attr = kcalloc(evt_num_attrs, sizeof(**evt_attr), GFP_KERNEL);
+
+	if (!evt_attr)

I think you want to check *evt_attr

+		goto err_evt_attr;
+
+	evt_attr_group->attrs = kcalloc(evt_num_attrs + 1,
+				sizeof(*evt_attr_group->attrs), GFP_KERNEL);
+
+	if (!evt_attr_group->attrs)
+		goto err_evt_attr_grp;
+
+	return 0;
+err_evt_attr_grp:
+	kfree(*evt_attr);
+err_evt_attr:
+	kfree(fmt_attr_group->attrs);
+err_fmt_attr_grp:
+	kfree(*fmt_attr);
+	return -ENOMEM;
 }
 
 /* init amdgpu_pmu */
 int amdgpu_pmu_init(struct amdgpu_device *adev)
 {
+	struct amdgpu_pmu_event_attribute *fmt_attr = NULL, *evt_attr = NULL,
+				*old_fmt_attr = NULL, *old_evt_attr = NULL;
 	int ret = 0;
 
+	if (adev->asic_type != CHIP_VEGA20)
+		goto init_events;
+
+	ret = amdgpu_pmu_alloc_pmu_attrs(&df_vega20_format_attr_group,
+					&old_fmt_attr,
+					NUM_FORMATS_DF_LEGACY,
+					&df_vega20_event_attr_group,
+					&old_evt_attr,
+					NUM_EVENTS_DF_LEGACY);
+
+	if (ret)
+		return ret;
+
+	ret = init_pmu_by_type(adev,
+				&df_vega20_format_attr_group, old_fmt_attr,
+				&df_vega20_event_attr_group, old_evt_attr,
+				"DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF_LEGACY);
+	if (ret)
+		goto err_old_pmu;
+
+init_events:
 	switch (adev->asic_type) {
 	case CHIP_VEGA20:
-		/* init df */
-		ret = init_pmu_by_type(adev, df_v3_6_attr_groups,
-				       "DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF,
-				       DF_V3_6_MAX_COUNTERS);
+		ret = amdgpu_pmu_alloc_pmu_attrs(&amdgpu_pmu_format_attr_group,
+						&fmt_attr,
+						NUM_FORMATS_AMDGPU_PMU,
+						&vega20_event_attr_group,
+						&evt_attr,
+						NUM_EVENTS_VEGA20_MAX);
+
+		if (ret)
+			goto err_alloc;
+
+		ret = init_pmu_by_type(adev,
+				&amdgpu_pmu_format_attr_group, fmt_attr,
+				&vega20_event_attr_group, evt_attr,
+				"Event", "amdgpu", PERF_TYPE_AMDGPU_MAX);
+
+		if (ret) {
+			kfree(vega20_event_attr_group.attrs);
+			goto err_pmu;
+		}
 
-		/* other pmu types go here*/
 		break;
 	default:
 		return 0;
-	}
+	};
 
 	return 0;
-}
-
-
-/* destroy all pmu data associated with target device */
-void amdgpu_pmu_fini(struct amdgpu_device *adev)
-{
-	struct amdgpu_pmu_entry *pe, *temp;
-
-	list_for_each_entry_safe(pe, temp, &amdgpu_pmu_list, entry) {
-		if (pe->adev == adev) {
-			list_del(&pe->entry);
-			perf_pmu_unregister(&pe->pmu);
-			kfree(pe);
-		}
-	}
+err_pmu:
+	kfree(fmt_attr);
+	kfree(evt_attr);
+	kfree(amdgpu_pmu_format_attr_group.attrs);
+err_alloc:
+	if (adev->asic_type == CHIP_VEGA20)
+		amdgpu_pmu_fini(adev);
+	return ret;
+err_old_pmu:
+	kfree(old_fmt_attr);
+	kfree(old_evt_attr);
+	kfree(df_vega20_format_attr_group.attrs);
+	kfree(df_vega20_event_attr_group.attrs);
+	return ret;
 }
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.h
index 7dddb7160a11..0d214abe720e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.h
@@ -27,10 +27,14 @@
 #define _AMDGPU_PMU_H_
 
 enum amdgpu_pmu_perf_type {
-	PERF_TYPE_AMDGPU_DF = 0,
+	PERF_TYPE_AMDGPU_DF_LEGACY = 0,
+	PERF_TYPE_AMDGPU_XGMI,
 	PERF_TYPE_AMDGPU_MAX
 };
 
+#define AMDGPU_PERF_TYPE_SHIFT	56
+#define AMDGPU_PERF_TYPE_MASK	0xff
+
 int amdgpu_pmu_init(struct amdgpu_device *adev);
 void amdgpu_pmu_fini(struct amdgpu_device *adev);
 
diff --git a/drivers/gpu/drm/amd/amdgpu/df_v3_6.c b/drivers/gpu/drm/amd/amdgpu/df_v3_6.c
index 0ca6e176acb0..6e57ae95f997 100644
--- a/drivers/gpu/drm/amd/amdgpu/df_v3_6.c
+++ b/drivers/gpu/drm/amd/amdgpu/df_v3_6.c
@@ -30,71 +30,17 @@
 #define DF_3_6_SMN_REG_INST_DIST        0x8
 #define DF_3_6_INST_CNT                 8
 
-static u32 df_v3_6_channel_number[] = {1, 2, 0, 4, 0, 8, 0,
-				       16, 32, 0, 0, 0, 2, 4, 8};
-
-/* init df format attrs */
-AMDGPU_PMU_ATTR(event,		"config:0-7");
-AMDGPU_PMU_ATTR(instance,	"config:8-15");
-AMDGPU_PMU_ATTR(umask,		"config:16-23");
-
-/* df format attributes  */
-static struct attribute *df_v3_6_format_attrs[] = {
-	&pmu_attr_event.attr,
-	&pmu_attr_instance.attr,
-	&pmu_attr_umask.attr,
-	NULL
-};
-
-/* df format attribute group */
-static struct attribute_group df_v3_6_format_attr_group = {
-	.name = "format",
-	.attrs = df_v3_6_format_attrs,
-};
+/* Defined in global_features.h as FTI_PERFMON_VISIBLE */
+#define DF_V3_6_MAX_COUNTERS		4
 
-/* df event attrs */
-AMDGPU_PMU_ATTR(cake0_pcsout_txdata,
-		      "event=0x7,instance=0x46,umask=0x2");
-AMDGPU_PMU_ATTR(cake1_pcsout_txdata,
-		      "event=0x7,instance=0x47,umask=0x2");
-AMDGPU_PMU_ATTR(cake0_pcsout_txmeta,
-		      "event=0x7,instance=0x46,umask=0x4");
-AMDGPU_PMU_ATTR(cake1_pcsout_txmeta,
-		      "event=0x7,instance=0x47,umask=0x4");
-AMDGPU_PMU_ATTR(cake0_ftiinstat_reqalloc,
-		      "event=0xb,instance=0x46,umask=0x4");
-AMDGPU_PMU_ATTR(cake1_ftiinstat_reqalloc,
-		      "event=0xb,instance=0x47,umask=0x4");
-AMDGPU_PMU_ATTR(cake0_ftiinstat_rspalloc,
-		      "event=0xb,instance=0x46,umask=0x8");
-AMDGPU_PMU_ATTR(cake1_ftiinstat_rspalloc,
-		      "event=0xb,instance=0x47,umask=0x8");
-
-/* df event attributes  */
-static struct attribute *df_v3_6_event_attrs[] = {
-	&pmu_attr_cake0_pcsout_txdata.attr,
-	&pmu_attr_cake1_pcsout_txdata.attr,
-	&pmu_attr_cake0_pcsout_txmeta.attr,
-	&pmu_attr_cake1_pcsout_txmeta.attr,
-	&pmu_attr_cake0_ftiinstat_reqalloc.attr,
-	&pmu_attr_cake1_ftiinstat_reqalloc.attr,
-	&pmu_attr_cake0_ftiinstat_rspalloc.attr,
-	&pmu_attr_cake1_ftiinstat_rspalloc.attr,
-	NULL
-};
-
-/* df event attribute group */
-static struct attribute_group df_v3_6_event_attr_group = {
-	.name = "events",
-	.attrs = df_v3_6_event_attrs
-};
+/* get flags from df perfmon config */
+#define DF_V3_6_GET_EVENT(x)		(x & 0xFFUL)
+#define DF_V3_6_GET_INSTANCE(x)		((x >> 8) & 0xFFUL)
+#define DF_V3_6_GET_UNITMASK(x)		((x >> 16) & 0xFFUL)
+#define DF_V3_6_PERFMON_OVERFLOW	0xFFFFFFFFFFFFULL
 
-/* df event attr groups  */
-const struct attribute_group *df_v3_6_attr_groups[] = {
-		&df_v3_6_format_attr_group,
-		&df_v3_6_event_attr_group,
-		NULL
-};
+static u32 df_v3_6_channel_number[] = {1, 2, 0, 4, 0, 8, 0,
+				       16, 32, 0, 0, 0, 2, 4, 8};
 
 static uint64_t df_v3_6_get_fica(struct amdgpu_device *adev,
 				 uint32_t ficaa_val)
diff --git a/drivers/gpu/drm/amd/amdgpu/df_v3_6.h b/drivers/gpu/drm/amd/amdgpu/df_v3_6.h
index 76998541bc30..2505c7ef258a 100644
--- a/drivers/gpu/drm/amd/amdgpu/df_v3_6.h
+++ b/drivers/gpu/drm/amd/amdgpu/df_v3_6.h
@@ -35,15 +35,6 @@ enum DF_V3_6_MGCG {
 	DF_V3_6_MGCG_ENABLE_63_CYCLE_DELAY = 15
 };
 
-/* Defined in global_features.h as FTI_PERFMON_VISIBLE */
-#define DF_V3_6_MAX_COUNTERS		4
-
-/* get flags from df perfmon config */
-#define DF_V3_6_GET_EVENT(x)		(x & 0xFFUL)
-#define DF_V3_6_GET_INSTANCE(x)		((x >> 8) & 0xFFUL)
-#define DF_V3_6_GET_UNITMASK(x)		((x >> 16) & 0xFFUL)
-#define DF_V3_6_PERFMON_OVERFLOW	0xFFFFFFFFFFFFULL
-
 extern const struct attribute_group *df_v3_6_attr_groups[];
 extern const struct amdgpu_df_funcs df_v3_6_funcs;
 
-- 
2.17.1
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

RE: [PATCH 3/3] drm/amdgpu: add xgmi perfmons for arcturus

[Kasiviswanathan, Harish]

[AMD Official Use Only - Internal Distribution Only]

Reviewed-by: Harish Kasiviswanathan <Harish.Kasiviswanathan@amd.com>

-----Original Message-----
From: Kim, Jonathan <Jonathan.Kim@amd.com> 
Sent: Tuesday, September 15, 2020 6:00 PM
To: amd-gfx@lists.freedesktop.org
Cc: Kasiviswanathan, Harish <Harish.Kasiviswanathan@amd.com>; Kim, Jonathan <Jonathan.Kim@amd.com>; Kim, Jonathan <Jonathan.Kim@amd.com>
Subject: [PATCH 3/3] drm/amdgpu: add xgmi perfmons for arcturus

Add xgmi perfmons for Arcturus.

Signed-off-by: Jonathan Kim <Jonathan.Kim@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c | 55 +++++++++++++++++++++++++
 drivers/gpu/drm/amd/amdgpu/df_v3_6.c    |  3 ++
 2 files changed, 58 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
index f3d2ac0e88a7..ec521c72e631 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
@@ -34,6 +34,8 @@
 #define NUM_EVENTS_DF_LEGACY		8
 #define NUM_EVENTS_VEGA20_XGMI		2
 #define NUM_EVENTS_VEGA20_MAX		2
+#define NUM_EVENTS_ARCTURUS_XGMI	6
+#define NUM_EVENTS_ARCTURUS_MAX		6
 
 /* record to keep track of pmu entry per pmu type per device */  struct amdgpu_pmu_entry { @@ -110,6 +112,27 @@ const struct attribute_group *vega20_attr_groups[] = {
 	NULL
 };
 
+/* Arcturus events */
+static const char *arcturus_events[NUM_EVENTS_ARCTURUS_MAX][2] = {
+	{ "xgmi_link0_data_outbound", "event=0x7,instance=0x4b,umask=0x2" },
+	{ "xgmi_link1_data_outbound", "event=0x7,instance=0x4c,umask=0x2" },
+	{ "xgmi_link2_data_outbound", "event=0x7,instance=0x4d,umask=0x2" },
+	{ "xgmi_link3_data_outbound", "event=0x7,instance=0x4e,umask=0x2" },
+	{ "xgmi_link4_data_outbound", "event=0x7,instance=0x4f,umask=0x2" },
+	{ "xgmi_link5_data_outbound", "event=0x7,instance=0x50,umask=0x2" } };
+
+static struct attribute_group arcturus_event_attr_group = {
+	.name = "events",
+	.attrs = NULL
+};
+
+const struct attribute_group *arcturus_attr_groups[] = {
+	&amdgpu_pmu_format_attr_group,
+	&arcturus_event_attr_group,
+	NULL
+};
+
 /* All df_vega20_* items are DEPRECATED. Use vega20_ items above instead. */  static const char *df_vega20_formats[NUM_FORMATS_DF_LEGACY][2] = {
 	{ "event", "config:0-7" },
@@ -400,6 +423,16 @@ static int init_pmu_by_type(struct amdgpu_device *adev,
 
 		pmu_entry->pmu.attr_groups = vega20_attr_groups;
 		break;
+	case CHIP_ARCTURUS:
+		amdgpu_pmu_create_attributes(evt_attr_group, evt_attr,
+				arcturus_events, 0, NUM_EVENTS_ARCTURUS_XGMI,
+				PERF_TYPE_AMDGPU_XGMI);
+		num_events += NUM_EVENTS_ARCTURUS_XGMI;
+
+		/* other events can be added here */
+
+		pmu_entry->pmu.attr_groups = arcturus_attr_groups;
+		break;
 	default:
 		return -ENODEV;
 	};
@@ -530,6 +563,28 @@ int amdgpu_pmu_init(struct amdgpu_device *adev)
 			goto err_pmu;
 		}
 
+		break;
+	case CHIP_ARCTURUS:
+		ret = amdgpu_pmu_alloc_pmu_attrs(&amdgpu_pmu_format_attr_group,
+						&fmt_attr,
+						NUM_FORMATS_AMDGPU_PMU,
+						&arcturus_event_attr_group,
+						&evt_attr,
+						NUM_EVENTS_ARCTURUS_MAX);
+
+		if (ret)
+			goto err_alloc;
+
+		ret = init_pmu_by_type(adev,
+				&amdgpu_pmu_format_attr_group, fmt_attr,
+				&arcturus_event_attr_group, evt_attr,
+				"Event", "amdgpu", PERF_TYPE_AMDGPU_MAX);
+
+		if (ret) {
+			kfree(arcturus_event_attr_group.attrs);
+			goto err_pmu;
+		}
+
 		break;
 	default:
 		return 0;
diff --git a/drivers/gpu/drm/amd/amdgpu/df_v3_6.c b/drivers/gpu/drm/amd/amdgpu/df_v3_6.c
index 6e57ae95f997..6b4b30a8dce5 100644
--- a/drivers/gpu/drm/amd/amdgpu/df_v3_6.c
+++ b/drivers/gpu/drm/amd/amdgpu/df_v3_6.c
@@ -513,6 +513,7 @@ static int df_v3_6_pmc_start(struct amdgpu_device *adev, uint64_t config,
 
 	switch (adev->asic_type) {
 	case CHIP_VEGA20:
+	case CHIP_ARCTURUS:
 		if (is_add)
 			return df_v3_6_pmc_add_cntr(adev, config);
 
@@ -554,6 +555,7 @@ static int df_v3_6_pmc_stop(struct amdgpu_device *adev, uint64_t config,
 
 	switch (adev->asic_type) {
 	case CHIP_VEGA20:
+	case CHIP_ARCTURUS:
 		ret = df_v3_6_pmc_get_ctrl_settings(adev,
 			config,
 			counter_idx,
@@ -590,6 +592,7 @@ static void df_v3_6_pmc_get_count(struct amdgpu_device *adev,
 
 	switch (adev->asic_type) {
 	case CHIP_VEGA20:
+	case CHIP_ARCTURUS:
 		df_v3_6_pmc_get_read_settings(adev, config, counter_idx,
 						&lo_base_addr, &hi_base_addr);
 
--
2.17.1
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

RE: [PATCH 2/3] drm/amdgpu: add per device user friendly xgmi events for vega20

[Kim, Jonathan]

> -----Original Message-----
> From: Kasiviswanathan, Harish <Harish.Kasiviswanathan@amd.com>
> Sent: Tuesday, September 15, 2020 10:08 PM
> To: Kim, Jonathan <Jonathan.Kim@amd.com>; amd-
> gfx@lists.freedesktop.org
> Subject: RE: [PATCH 2/3] drm/amdgpu: add per device user friendly xgmi
> events for vega20
> 
> [AMD Official Use Only - Internal Distribution Only]
> 
> Some comments and a question
> 
> -----Original Message-----
> From: Kim, Jonathan <Jonathan.Kim@amd.com>
> Sent: Tuesday, September 15, 2020 6:00 PM
> To: amd-gfx@lists.freedesktop.org
> Cc: Kasiviswanathan, Harish <Harish.Kasiviswanathan@amd.com>; Kim,
> Jonathan <Jonathan.Kim@amd.com>; Kim, Jonathan
> <Jonathan.Kim@amd.com>
> Subject: [PATCH 2/3] drm/amdgpu: add per device user friendly xgmi events
> for vega20
> 
> Non-outbound data metrics are non useful so mark them as legacy.
> Bucket new perf counters into device and not device ip.
> Bind events to chip instead of IP.
> Report available event counters and not number of hw counter banks.
> Move DF public macros to private since not needed outside of IP version.
> 
> v2: add comments on sysfs structure and formatting.
> 
> Signed-off-by: Jonathan Kim <Jonathan.Kim@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu.h     |  13 -
>  drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c | 342
> +++++++++++++++++++++---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.h |   6 +-
>  drivers/gpu/drm/amd/amdgpu/df_v3_6.c    |  72 +----
>  drivers/gpu/drm/amd/amdgpu/df_v3_6.h    |   9 -
>  5 files changed, 314 insertions(+), 128 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index 13f92dea182a..f43dfdd2716a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -1279,19 +1279,6 @@ bool amdgpu_device_load_pci_state(struct
> pci_dev *pdev);
> 
>  #include "amdgpu_object.h"
> 
> -/* used by df_v3_6.c and amdgpu_pmu.c */
> -#define AMDGPU_PMU_ATTR(_name, _object)
> 		\
> -static ssize_t								\
> -_name##_show(struct device *dev,					\
> -			       struct device_attribute *attr,		\
> -			       char *page)				\
> -{									\
> -	BUILD_BUG_ON(sizeof(_object) >= PAGE_SIZE - 1);
> 	\
> -	return sprintf(page, _object "\n");				\
> -}									\
> -									\
> -static struct device_attribute pmu_attr_##_name = __ATTR_RO(_name)
> -
>  static inline bool amdgpu_is_tmz(struct amdgpu_device *adev)  {
>         return adev->gmc.tmz_enabled;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
> index 1b0ec715c8ba..f3d2ac0e88a7 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
> @@ -27,9 +27,13 @@
>  #include <linux/init.h>
>  #include "amdgpu.h"
>  #include "amdgpu_pmu.h"
> -#include "df_v3_6.h"
> 
>  #define PMU_NAME_SIZE 32
> +#define NUM_FORMATS_AMDGPU_PMU		4
> +#define NUM_FORMATS_DF_LEGACY		3
> +#define NUM_EVENTS_DF_LEGACY		8
> +#define NUM_EVENTS_VEGA20_XGMI		2
> +#define NUM_EVENTS_VEGA20_MAX		2
> 
>  /* record to keep track of pmu entry per pmu type per device */  struct
> amdgpu_pmu_entry { @@ -39,8 +43,106 @@ struct amdgpu_pmu_entry {
>  	unsigned int pmu_perf_type;
>  };
> 
> +struct amdgpu_pmu_event_attribute {
> +	struct device_attribute attr;
> +	const char *event_str;
> +	unsigned int type;
> +};
> +
> +static ssize_t amdgpu_pmu_event_show(struct device *dev,
> +				struct device_attribute *attr, char *buf) {
> +	struct amdgpu_pmu_event_attribute *amdgpu_pmu_attr;
> +
> +	amdgpu_pmu_attr = container_of(attr, struct
> amdgpu_pmu_event_attribute,
> +									attr);
> +
> +	if (!amdgpu_pmu_attr->type)
> +		return sprintf(buf, "%s\n", amdgpu_pmu_attr->event_str);
> +
> +	return sprintf(buf, "%s,type=0x%x\n",
> +			amdgpu_pmu_attr->event_str, amdgpu_pmu_attr-
> >type); }
> +
> +static struct attribute_group amdgpu_pmu_format_attr_group = {
> +	.name = "format",
> +	.attrs = NULL,
> +};
> +
> +/*
> + * Event formatting is global to all amdgpu events under sysfs folder
> + * /sys/bus/event_source/devices/amdgpu_<dev_num> where dev_num is
> the
> + * primary device index. Registered events can be found in subfolder
> "events"
> + * and formatting under subfolder "format".
> + *
> + * Formats "event", "instance", and "umask" are currently used by xGMI
> +but can
> + * be for generalized for other IP usage.  If format naming is
> +insufficient
> + * for newly registered IP events, append to the list below and handle
> +the
> + * perf events hardware configuration (see hwc->config) as required by the
> IP.
> + *
> + * Format "type" indicates IP type generated on pmu registration (see
> + * init_pmu_by_type) so non-legacy events omit this in the per-chip
> +event
> + * list (e.g. vega20_events).
> + */
> +static const char
> *amdgpu_pmu_formats[NUM_FORMATS_AMDGPU_PMU][2] = {
> +	{ "event", "config:0-7" },
> +	{ "instance", "config:8-15" },
> +	{ "umask", "config:16-23"},
> +	{ "type", "config:56-63"}
> +};
> +
>  static LIST_HEAD(amdgpu_pmu_list);
> 
> +/* Vega20 events */
> +static const char *vega20_events[NUM_EVENTS_VEGA20_MAX][2] = {
> +	{ "xgmi_link0_data_outbound",
> "event=0x7,instance=0x46,umask=0x2" },
> +	{ "xgmi_link1_data_outbound",
> "event=0x7,instance=0x47,umask=0x2" } };
> +
> +static struct attribute_group vega20_event_attr_group = {
> +	.name = "events",
> +	.attrs = NULL
> +};
> +
> +const struct attribute_group *vega20_attr_groups[] = {
> +	&amdgpu_pmu_format_attr_group,
> +	&vega20_event_attr_group,
> +	NULL
> +};
> +
> +/* All df_vega20_* items are DEPRECATED. Use vega20_ items above
> +instead. */ static const char
> *df_vega20_formats[NUM_FORMATS_DF_LEGACY][2] = {
> +	{ "event", "config:0-7" },
> +	{ "instance", "config:8-15" },
> +	{ "umask", "config:16-23"}
> +};
> +
> +static const char *df_vega20_events[NUM_EVENTS_DF_LEGACY][2] = {
> +	{ "cake0_pcsout_txdata", "event=0x7,instance=0x46,umask=0x2" },
> +	{ "cake1_pcsout_txdata", "event=0x7,instance=0x47,umask=0x2" },
> +	{ "cake0_pcsout_txmeta", "event=0x7,instance=0x46,umask=0x4" },
> +	{ "cake1_pcsout_txmeta", "event=0x7,instance=0x47,umask=0x4" },
> +	{ "cake0_ftiinstat_reqalloc", "event=0xb,instance=0x46,umask=0x4" },
> +	{ "cake1_ftiinstat_reqalloc", "event=0xb,instance=0x47,umask=0x4" },
> +	{ "cake0_ftiinstat_rspalloc", "event=0xb,instance=0x46,umask=0x8" },
> +	{ "cake1_ftiinstat_rspalloc", "event=0xb,instance=0x47,umask=0x8" },
> +};
> +
> +static struct attribute_group df_vega20_format_attr_group = {
> +	.name = "format",
> +	.attrs = NULL,
> +};
> +
> +static struct attribute_group df_vega20_event_attr_group = {
> +	.name = "events",
> +	.attrs = NULL
> +};
> +
> +const struct attribute_group *df_vega20_attr_groups[] = {
> +	&df_vega20_format_attr_group,
> +	&df_vega20_event_attr_group,
> +	NULL
> +};
> 
>  /* initialize perf counter */
>  static int amdgpu_perf_event_init(struct perf_event *event) @@ -73,7
> +175,8 @@ static void amdgpu_perf_start(struct perf_event *event, int
> flags)
>  	hwc->state = 0;
> 
>  	switch (pe->pmu_perf_type) {
> -	case PERF_TYPE_AMDGPU_DF:
> +	case PERF_TYPE_AMDGPU_DF_LEGACY:
> +	case PERF_TYPE_AMDGPU_XGMI:
>  		if (!(flags & PERF_EF_RELOAD)) {
>  			target_cntr = pe->adev->df.funcs->pmc_start(pe-
> >adev,
>  						hwc->config, 0 /* unused */,
> @@ -108,7 +211,8 @@ static void amdgpu_perf_read(struct perf_event
> *event)
>  		prev = local64_read(&hwc->prev_count);
> 
>  		switch (pe->pmu_perf_type) {
> -		case PERF_TYPE_AMDGPU_DF:
> +		case PERF_TYPE_AMDGPU_DF_LEGACY:
> +		case PERF_TYPE_AMDGPU_XGMI:
>  			pe->adev->df.funcs->pmc_get_count(pe->adev,
>  						hwc->config, hwc->idx,
> &count);
>  			break;
> @@ -133,7 +237,8 @@ static void amdgpu_perf_stop(struct perf_event
> *event, int flags)
>  		return;
> 
>  	switch (pe->pmu_perf_type) {
> -	case PERF_TYPE_AMDGPU_DF:
> +	case PERF_TYPE_AMDGPU_DF_LEGACY:
> +	case PERF_TYPE_AMDGPU_XGMI:
>  		pe->adev->df.funcs->pmc_stop(pe->adev, hwc->config, hwc-
> >idx,
>  									0);
>  		break;
> @@ -160,10 +265,15 @@ static int amdgpu_perf_add(struct perf_event
> *event, int flags)
>  						  struct amdgpu_pmu_entry,
>  						  pmu);
> 
> +	if (pe->pmu_perf_type == PERF_TYPE_AMDGPU_MAX)
> +		pe->pmu_perf_type = (hwc->config >>
> AMDGPU_PERF_TYPE_SHIFT) &
> +
> 	AMDGPU_PERF_TYPE_MASK;
> +
>  	event->hw.state = PERF_HES_UPTODATE | PERF_HES_STOPPED;
> 
>  	switch (pe->pmu_perf_type) {
> -	case PERF_TYPE_AMDGPU_DF:
> +	case PERF_TYPE_AMDGPU_DF_LEGACY:
> +	case PERF_TYPE_AMDGPU_XGMI:
>  		target_cntr = pe->adev->df.funcs->pmc_start(pe->adev,
>  						hwc->config, 0 /* unused */,
>  						1 /* add counter */);
> @@ -197,7 +307,8 @@ static void amdgpu_perf_del(struct perf_event
> *event, int flags)
>  	amdgpu_perf_stop(event, PERF_EF_UPDATE);
> 
>  	switch (pe->pmu_perf_type) {
> -	case PERF_TYPE_AMDGPU_DF:
> +	case PERF_TYPE_AMDGPU_DF_LEGACY:
> +	case PERF_TYPE_AMDGPU_XGMI:
>  		pe->adev->df.funcs->pmc_stop(pe->adev, hwc->config, hwc-
> >idx,
>  									1);
>  		break;
> @@ -208,18 +319,42 @@ static void amdgpu_perf_del(struct perf_event
> *event, int flags)
>  	perf_event_update_userpage(event);
>  }
> 
> -/* vega20 pmus */
> +static void amdgpu_pmu_create_attributes(struct attribute_group
> *attr_group,
> +				struct amdgpu_pmu_event_attribute
> *pmu_attr,
> +				const char *events[][2],
> +				int s_offset,
> +				int e_offset,
> +				unsigned int type)
> +{
> +	int i;
> +
> +	pmu_attr += s_offset;
> +
> +	for (i = s_offset; i < e_offset; i++) {
> +		attr_group->attrs[i] = &pmu_attr->attr.attr;
> +		sysfs_attr_init(&pmu_attr->attr.attr);
> +		pmu_attr->attr.attr.name = events[i][0];
> +		pmu_attr->attr.attr.mode = 0444;
> +		pmu_attr->attr.show = amdgpu_pmu_event_show;
> +		pmu_attr->event_str = events[i][1];
> +		pmu_attr->type = type;
> +		pmu_attr++;
> +	}
> +}
> 
>  /* init pmu tracking per pmu type */
>  static int init_pmu_by_type(struct amdgpu_device *adev,
> -		  const struct attribute_group *attr_groups[],
> -		  char *pmu_type_name, char *pmu_file_prefix,
> -		  unsigned int pmu_perf_type,
> -		  unsigned int num_counters)
> +			struct attribute_group *fmt_attr_group,
> +			struct amdgpu_pmu_event_attribute *fmt_attr,
> +			struct attribute_group *evt_attr_group,
> +			struct amdgpu_pmu_event_attribute *evt_attr,
> +			char *pmu_type_name, char *pmu_file_prefix,
> +			unsigned int pmu_perf_type)
>  {
>  	char pmu_name[PMU_NAME_SIZE];
>  	struct amdgpu_pmu_entry *pmu_entry;
> -	int ret = 0;
> +	bool is_legacy = pmu_perf_type ==
> PERF_TYPE_AMDGPU_DF_LEGACY;
> +	int ret = 0, num_events = 0;
> 
>  	pmu_entry = kzalloc(sizeof(struct amdgpu_pmu_entry),
> GFP_KERNEL);
> 
> @@ -237,59 +372,182 @@ static int init_pmu_by_type(struct
> amdgpu_device *adev,
>  		.task_ctx_nr = perf_invalid_context,
>  	};
> 
> -	pmu_entry->pmu.attr_groups = attr_groups;
> +	amdgpu_pmu_create_attributes(fmt_attr_group, fmt_attr,
> +			is_legacy ? df_vega20_formats :
> amdgpu_pmu_formats, 0,
> +			is_legacy ? NUM_FORMATS_DF_LEGACY :
> +
> 	NUM_FORMATS_AMDGPU_PMU,
> +			0);
> +
> +	if (is_legacy) {
> +		amdgpu_pmu_create_attributes(evt_attr_group, evt_attr,
> +					df_vega20_events, 0,
> +					NUM_EVENTS_DF_LEGACY,
> +					PERF_TYPE_AMDGPU_DF_LEGACY);
> +		num_events += NUM_EVENTS_DF_LEGACY;
> +
> +		pmu_entry->pmu.attr_groups = df_vega20_attr_groups;
> +		goto legacy_register;
> +	}
> +
> +	switch (adev->asic_type) {
> +	case CHIP_VEGA20:
> +		amdgpu_pmu_create_attributes(evt_attr_group, evt_attr,
> +				vega20_events, 0,
> NUM_EVENTS_VEGA20_XGMI,
> +				PERF_TYPE_AMDGPU_XGMI);
> +		num_events += NUM_EVENTS_VEGA20_XGMI;
> +
> +		/* other events can be added here */
> +
> +		pmu_entry->pmu.attr_groups = vega20_attr_groups;
> +		break;
> +	default:
> +		return -ENODEV;
> +	};
> +
> +legacy_register:
>  	pmu_entry->pmu_perf_type = pmu_perf_type;
> -	snprintf(pmu_name, PMU_NAME_SIZE, "%s_%d",
> -				pmu_file_prefix, adev_to_drm(adev)-
> >primary->index);
> +	snprintf(pmu_name, PMU_NAME_SIZE, "%s_%d", pmu_file_prefix,
> +					adev_to_drm(adev)->primary-
> >index);
> 
>  	ret = perf_pmu_register(&pmu_entry->pmu, pmu_name, -1);
> 
> -	if (ret) {
> -		kfree(pmu_entry);
> -		pr_warn("Error initializing AMDGPU %s PMUs.\n",
> pmu_type_name);
> -		return ret;
> -	}
> +	if (ret)
> +		goto err;
> 
>  	pr_info("Detected AMDGPU %s Counters. # of Counters = %d.\n",
> -			pmu_type_name, num_counters);
> +			pmu_type_name, num_events);
> 
>  	list_add_tail(&pmu_entry->entry, &amdgpu_pmu_list);
> 
>  	return 0;
> +err:
> +	kfree(pmu_entry);
> +	pr_warn("Error initializing AMDGPU %s PMUs.\n", pmu_type_name);
> +	return ret;
> +}
> +
> +/* destroy all pmu data associated with target device */ void
> +amdgpu_pmu_fini(struct amdgpu_device *adev) {
> +	struct amdgpu_pmu_entry *pe, *temp;
> +
> +	list_for_each_entry_safe(pe, temp, &amdgpu_pmu_list, entry) {
> +		if (pe->adev == adev) {
> +			list_del(&pe->entry);
> +			perf_pmu_unregister(&pe->pmu);
> +			kfree(pe);
> +		}
> +	}
> +}
> +
> +static int amdgpu_pmu_alloc_pmu_attrs(
> +				struct attribute_group *fmt_attr_group,
> +				struct amdgpu_pmu_event_attribute
> **fmt_attr,
> +				int fmt_num_attrs,
> +				struct attribute_group *evt_attr_group,
> +				struct amdgpu_pmu_event_attribute
> **evt_attr,
> +				int evt_num_attrs)
> +{
> 
> where do all these allocations get freed?

Thanks for the mem leak catch.  I'll free these up in amdgpu_pmu_fini after pmu unregister and fix the bad ENOMEM condition checks below.

Jon

> 
> +	*fmt_attr = kcalloc(fmt_num_attrs, sizeof(**fmt_attr),
> GFP_KERNEL);
> +
> +	if (!fmt_attr)
> 
> 
> I think you want to check *fmt_attr
> 
> +		return -ENOMEM;
> +
> +	fmt_attr_group->attrs = kcalloc(fmt_num_attrs + 1,
> +				sizeof(*fmt_attr_group->attrs),
> GFP_KERNEL);
> +
> +	if (!fmt_attr_group->attrs)
> +		goto err_fmt_attr_grp;
> +
> +	*evt_attr = kcalloc(evt_num_attrs, sizeof(**evt_attr), GFP_KERNEL);
> +
> +	if (!evt_attr)
> 
> I think you want to check *evt_attr
> 
> +		goto err_evt_attr;
> +
> +	evt_attr_group->attrs = kcalloc(evt_num_attrs + 1,
> +				sizeof(*evt_attr_group->attrs), GFP_KERNEL);
> +
> +	if (!evt_attr_group->attrs)
> +		goto err_evt_attr_grp;
> +
> +	return 0;
> +err_evt_attr_grp:
> +	kfree(*evt_attr);
> +err_evt_attr:
> +	kfree(fmt_attr_group->attrs);
> +err_fmt_attr_grp:
> +	kfree(*fmt_attr);
> +	return -ENOMEM;
>  }
> 
>  /* init amdgpu_pmu */
>  int amdgpu_pmu_init(struct amdgpu_device *adev)  {
> +	struct amdgpu_pmu_event_attribute *fmt_attr = NULL, *evt_attr =
> NULL,
> +				*old_fmt_attr = NULL, *old_evt_attr = NULL;
>  	int ret = 0;
> 
> +	if (adev->asic_type != CHIP_VEGA20)
> +		goto init_events;
> +
> +	ret =
> amdgpu_pmu_alloc_pmu_attrs(&df_vega20_format_attr_group,
> +					&old_fmt_attr,
> +					NUM_FORMATS_DF_LEGACY,
> +					&df_vega20_event_attr_group,
> +					&old_evt_attr,
> +					NUM_EVENTS_DF_LEGACY);
> +
> +	if (ret)
> +		return ret;
> +
> +	ret = init_pmu_by_type(adev,
> +				&df_vega20_format_attr_group,
> old_fmt_attr,
> +				&df_vega20_event_attr_group, old_evt_attr,
> +				"DF", "amdgpu_df",
> PERF_TYPE_AMDGPU_DF_LEGACY);
> +	if (ret)
> +		goto err_old_pmu;
> +
> +init_events:
>  	switch (adev->asic_type) {
>  	case CHIP_VEGA20:
> -		/* init df */
> -		ret = init_pmu_by_type(adev, df_v3_6_attr_groups,
> -				       "DF", "amdgpu_df",
> PERF_TYPE_AMDGPU_DF,
> -				       DF_V3_6_MAX_COUNTERS);
> +		ret =
> amdgpu_pmu_alloc_pmu_attrs(&amdgpu_pmu_format_attr_group,
> +						&fmt_attr,
> +
> 	NUM_FORMATS_AMDGPU_PMU,
> +						&vega20_event_attr_group,
> +						&evt_attr,
> +
> 	NUM_EVENTS_VEGA20_MAX);
> +
> +		if (ret)
> +			goto err_alloc;
> +
> +		ret = init_pmu_by_type(adev,
> +				&amdgpu_pmu_format_attr_group,
> fmt_attr,
> +				&vega20_event_attr_group, evt_attr,
> +				"Event", "amdgpu",
> PERF_TYPE_AMDGPU_MAX);
> +
> +		if (ret) {
> +			kfree(vega20_event_attr_group.attrs);
> +			goto err_pmu;
> +		}
> 
> -		/* other pmu types go here*/
>  		break;
>  	default:
>  		return 0;
> -	}
> +	};
> 
>  	return 0;
> -}
> -
> -
> -/* destroy all pmu data associated with target device */ -void
> amdgpu_pmu_fini(struct amdgpu_device *adev) -{
> -	struct amdgpu_pmu_entry *pe, *temp;
> -
> -	list_for_each_entry_safe(pe, temp, &amdgpu_pmu_list, entry) {
> -		if (pe->adev == adev) {
> -			list_del(&pe->entry);
> -			perf_pmu_unregister(&pe->pmu);
> -			kfree(pe);
> -		}
> -	}
> +err_pmu:
> +	kfree(fmt_attr);
> +	kfree(evt_attr);
> +	kfree(amdgpu_pmu_format_attr_group.attrs);
> +err_alloc:
> +	if (adev->asic_type == CHIP_VEGA20)
> +		amdgpu_pmu_fini(adev);
> +	return ret;
> +err_old_pmu:
> +	kfree(old_fmt_attr);
> +	kfree(old_evt_attr);
> +	kfree(df_vega20_format_attr_group.attrs);
> +	kfree(df_vega20_event_attr_group.attrs);
> +	return ret;
>  }
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.h
> index 7dddb7160a11..0d214abe720e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.h
> @@ -27,10 +27,14 @@
>  #define _AMDGPU_PMU_H_
> 
>  enum amdgpu_pmu_perf_type {
> -	PERF_TYPE_AMDGPU_DF = 0,
> +	PERF_TYPE_AMDGPU_DF_LEGACY = 0,
> +	PERF_TYPE_AMDGPU_XGMI,
>  	PERF_TYPE_AMDGPU_MAX
>  };
> 
> +#define AMDGPU_PERF_TYPE_SHIFT	56
> +#define AMDGPU_PERF_TYPE_MASK	0xff
> +
>  int amdgpu_pmu_init(struct amdgpu_device *adev);  void
> amdgpu_pmu_fini(struct amdgpu_device *adev);
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/df_v3_6.c
> b/drivers/gpu/drm/amd/amdgpu/df_v3_6.c
> index 0ca6e176acb0..6e57ae95f997 100644
> --- a/drivers/gpu/drm/amd/amdgpu/df_v3_6.c
> +++ b/drivers/gpu/drm/amd/amdgpu/df_v3_6.c
> @@ -30,71 +30,17 @@
>  #define DF_3_6_SMN_REG_INST_DIST        0x8
>  #define DF_3_6_INST_CNT                 8
> 
> -static u32 df_v3_6_channel_number[] = {1, 2, 0, 4, 0, 8, 0,
> -				       16, 32, 0, 0, 0, 2, 4, 8};
> -
> -/* init df format attrs */
> -AMDGPU_PMU_ATTR(event,		"config:0-7");
> -AMDGPU_PMU_ATTR(instance,	"config:8-15");
> -AMDGPU_PMU_ATTR(umask,		"config:16-23");
> -
> -/* df format attributes  */
> -static struct attribute *df_v3_6_format_attrs[] = {
> -	&pmu_attr_event.attr,
> -	&pmu_attr_instance.attr,
> -	&pmu_attr_umask.attr,
> -	NULL
> -};
> -
> -/* df format attribute group */
> -static struct attribute_group df_v3_6_format_attr_group = {
> -	.name = "format",
> -	.attrs = df_v3_6_format_attrs,
> -};
> +/* Defined in global_features.h as FTI_PERFMON_VISIBLE */
> +#define DF_V3_6_MAX_COUNTERS		4
> 
> -/* df event attrs */
> -AMDGPU_PMU_ATTR(cake0_pcsout_txdata,
> -		      "event=0x7,instance=0x46,umask=0x2");
> -AMDGPU_PMU_ATTR(cake1_pcsout_txdata,
> -		      "event=0x7,instance=0x47,umask=0x2");
> -AMDGPU_PMU_ATTR(cake0_pcsout_txmeta,
> -		      "event=0x7,instance=0x46,umask=0x4");
> -AMDGPU_PMU_ATTR(cake1_pcsout_txmeta,
> -		      "event=0x7,instance=0x47,umask=0x4");
> -AMDGPU_PMU_ATTR(cake0_ftiinstat_reqalloc,
> -		      "event=0xb,instance=0x46,umask=0x4");
> -AMDGPU_PMU_ATTR(cake1_ftiinstat_reqalloc,
> -		      "event=0xb,instance=0x47,umask=0x4");
> -AMDGPU_PMU_ATTR(cake0_ftiinstat_rspalloc,
> -		      "event=0xb,instance=0x46,umask=0x8");
> -AMDGPU_PMU_ATTR(cake1_ftiinstat_rspalloc,
> -		      "event=0xb,instance=0x47,umask=0x8");
> -
> -/* df event attributes  */
> -static struct attribute *df_v3_6_event_attrs[] = {
> -	&pmu_attr_cake0_pcsout_txdata.attr,
> -	&pmu_attr_cake1_pcsout_txdata.attr,
> -	&pmu_attr_cake0_pcsout_txmeta.attr,
> -	&pmu_attr_cake1_pcsout_txmeta.attr,
> -	&pmu_attr_cake0_ftiinstat_reqalloc.attr,
> -	&pmu_attr_cake1_ftiinstat_reqalloc.attr,
> -	&pmu_attr_cake0_ftiinstat_rspalloc.attr,
> -	&pmu_attr_cake1_ftiinstat_rspalloc.attr,
> -	NULL
> -};
> -
> -/* df event attribute group */
> -static struct attribute_group df_v3_6_event_attr_group = {
> -	.name = "events",
> -	.attrs = df_v3_6_event_attrs
> -};
> +/* get flags from df perfmon config */
> +#define DF_V3_6_GET_EVENT(x)		(x & 0xFFUL)
> +#define DF_V3_6_GET_INSTANCE(x)		((x >> 8) & 0xFFUL)
> +#define DF_V3_6_GET_UNITMASK(x)		((x >> 16) & 0xFFUL)
> +#define DF_V3_6_PERFMON_OVERFLOW	0xFFFFFFFFFFFFULL
> 
> -/* df event attr groups  */
> -const struct attribute_group *df_v3_6_attr_groups[] = {
> -		&df_v3_6_format_attr_group,
> -		&df_v3_6_event_attr_group,
> -		NULL
> -};
> +static u32 df_v3_6_channel_number[] = {1, 2, 0, 4, 0, 8, 0,
> +				       16, 32, 0, 0, 0, 2, 4, 8};
> 
>  static uint64_t df_v3_6_get_fica(struct amdgpu_device *adev,
>  				 uint32_t ficaa_val)
> diff --git a/drivers/gpu/drm/amd/amdgpu/df_v3_6.h
> b/drivers/gpu/drm/amd/amdgpu/df_v3_6.h
> index 76998541bc30..2505c7ef258a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/df_v3_6.h
> +++ b/drivers/gpu/drm/amd/amdgpu/df_v3_6.h
> @@ -35,15 +35,6 @@ enum DF_V3_6_MGCG {
>  	DF_V3_6_MGCG_ENABLE_63_CYCLE_DELAY = 15  };
> 
> -/* Defined in global_features.h as FTI_PERFMON_VISIBLE */
> -#define DF_V3_6_MAX_COUNTERS		4
> -
> -/* get flags from df perfmon config */
> -#define DF_V3_6_GET_EVENT(x)		(x & 0xFFUL)
> -#define DF_V3_6_GET_INSTANCE(x)		((x >> 8) & 0xFFUL)
> -#define DF_V3_6_GET_UNITMASK(x)		((x >> 16) & 0xFFUL)
> -#define DF_V3_6_PERFMON_OVERFLOW	0xFFFFFFFFFFFFULL
> -
>  extern const struct attribute_group *df_v3_6_attr_groups[];  extern const
> struct amdgpu_df_funcs df_v3_6_funcs;
> 
> --
> 2.17.1
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

RE: [PATCH 3/3] drm/amdgpu: add xgmi perfmons for arcturus

[Kim, Jonathan]

Hi Harish.  Thanks for the review.  As discussed offline, patch 2 has a problem where attr groups array is global but allocation is done per-device causing problems with mem free and pmu unregister.  I'm sending out a second series that should fix this and simplify the solution as well as hopefully address your concerns.  Sorry again for the churn.

Jon

> -----Original Message-----
> From: Kasiviswanathan, Harish <Harish.Kasiviswanathan@amd.com>
> Sent: Tuesday, September 15, 2020 10:10 PM
> To: Kim, Jonathan <Jonathan.Kim@amd.com>; amd-
> gfx@lists.freedesktop.org
> Subject: RE: [PATCH 3/3] drm/amdgpu: add xgmi perfmons for arcturus
> 
> [AMD Official Use Only - Internal Distribution Only]
> 
> Reviewed-by: Harish Kasiviswanathan <Harish.Kasiviswanathan@amd.com>
> 
> -----Original Message-----
> From: Kim, Jonathan <Jonathan.Kim@amd.com>
> Sent: Tuesday, September 15, 2020 6:00 PM
> To: amd-gfx@lists.freedesktop.org
> Cc: Kasiviswanathan, Harish <Harish.Kasiviswanathan@amd.com>; Kim,
> Jonathan <Jonathan.Kim@amd.com>; Kim, Jonathan
> <Jonathan.Kim@amd.com>
> Subject: [PATCH 3/3] drm/amdgpu: add xgmi perfmons for arcturus
> 
> Add xgmi perfmons for Arcturus.
> 
> Signed-off-by: Jonathan Kim <Jonathan.Kim@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c | 55
> +++++++++++++++++++++++++
>  drivers/gpu/drm/amd/amdgpu/df_v3_6.c    |  3 ++
>  2 files changed, 58 insertions(+)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
> index f3d2ac0e88a7..ec521c72e631 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
> @@ -34,6 +34,8 @@
>  #define NUM_EVENTS_DF_LEGACY		8
>  #define NUM_EVENTS_VEGA20_XGMI		2
>  #define NUM_EVENTS_VEGA20_MAX		2
> +#define NUM_EVENTS_ARCTURUS_XGMI	6
> +#define NUM_EVENTS_ARCTURUS_MAX		6
> 
>  /* record to keep track of pmu entry per pmu type per device */  struct
> amdgpu_pmu_entry { @@ -110,6 +112,27 @@ const struct attribute_group
> *vega20_attr_groups[] = {
>  	NULL
>  };
> 
> +/* Arcturus events */
> +static const char *arcturus_events[NUM_EVENTS_ARCTURUS_MAX][2] = {
> +	{ "xgmi_link0_data_outbound",
> "event=0x7,instance=0x4b,umask=0x2" },
> +	{ "xgmi_link1_data_outbound",
> "event=0x7,instance=0x4c,umask=0x2" },
> +	{ "xgmi_link2_data_outbound",
> "event=0x7,instance=0x4d,umask=0x2" },
> +	{ "xgmi_link3_data_outbound",
> "event=0x7,instance=0x4e,umask=0x2" },
> +	{ "xgmi_link4_data_outbound",
> "event=0x7,instance=0x4f,umask=0x2" },
> +	{ "xgmi_link5_data_outbound",
> "event=0x7,instance=0x50,umask=0x2" } };
> +
> +static struct attribute_group arcturus_event_attr_group = {
> +	.name = "events",
> +	.attrs = NULL
> +};
> +
> +const struct attribute_group *arcturus_attr_groups[] = {
> +	&amdgpu_pmu_format_attr_group,
> +	&arcturus_event_attr_group,
> +	NULL
> +};
> +
>  /* All df_vega20_* items are DEPRECATED. Use vega20_ items above
> instead. */  static const char
> *df_vega20_formats[NUM_FORMATS_DF_LEGACY][2] = {
>  	{ "event", "config:0-7" },
> @@ -400,6 +423,16 @@ static int init_pmu_by_type(struct amdgpu_device
> *adev,
> 
>  		pmu_entry->pmu.attr_groups = vega20_attr_groups;
>  		break;
> +	case CHIP_ARCTURUS:
> +		amdgpu_pmu_create_attributes(evt_attr_group, evt_attr,
> +				arcturus_events, 0,
> NUM_EVENTS_ARCTURUS_XGMI,
> +				PERF_TYPE_AMDGPU_XGMI);
> +		num_events += NUM_EVENTS_ARCTURUS_XGMI;
> +
> +		/* other events can be added here */
> +
> +		pmu_entry->pmu.attr_groups = arcturus_attr_groups;
> +		break;
>  	default:
>  		return -ENODEV;
>  	};
> @@ -530,6 +563,28 @@ int amdgpu_pmu_init(struct amdgpu_device *adev)
>  			goto err_pmu;
>  		}
> 
> +		break;
> +	case CHIP_ARCTURUS:
> +		ret =
> amdgpu_pmu_alloc_pmu_attrs(&amdgpu_pmu_format_attr_group,
> +						&fmt_attr,
> +
> 	NUM_FORMATS_AMDGPU_PMU,
> +						&arcturus_event_attr_group,
> +						&evt_attr,
> +
> 	NUM_EVENTS_ARCTURUS_MAX);
> +
> +		if (ret)
> +			goto err_alloc;
> +
> +		ret = init_pmu_by_type(adev,
> +				&amdgpu_pmu_format_attr_group,
> fmt_attr,
> +				&arcturus_event_attr_group, evt_attr,
> +				"Event", "amdgpu",
> PERF_TYPE_AMDGPU_MAX);
> +
> +		if (ret) {
> +			kfree(arcturus_event_attr_group.attrs);
> +			goto err_pmu;
> +		}
> +
>  		break;
>  	default:
>  		return 0;
> diff --git a/drivers/gpu/drm/amd/amdgpu/df_v3_6.c
> b/drivers/gpu/drm/amd/amdgpu/df_v3_6.c
> index 6e57ae95f997..6b4b30a8dce5 100644
> --- a/drivers/gpu/drm/amd/amdgpu/df_v3_6.c
> +++ b/drivers/gpu/drm/amd/amdgpu/df_v3_6.c
> @@ -513,6 +513,7 @@ static int df_v3_6_pmc_start(struct amdgpu_device
> *adev, uint64_t config,
> 
>  	switch (adev->asic_type) {
>  	case CHIP_VEGA20:
> +	case CHIP_ARCTURUS:
>  		if (is_add)
>  			return df_v3_6_pmc_add_cntr(adev, config);
> 
> @@ -554,6 +555,7 @@ static int df_v3_6_pmc_stop(struct amdgpu_device
> *adev, uint64_t config,
> 
>  	switch (adev->asic_type) {
>  	case CHIP_VEGA20:
> +	case CHIP_ARCTURUS:
>  		ret = df_v3_6_pmc_get_ctrl_settings(adev,
>  			config,
>  			counter_idx,
> @@ -590,6 +592,7 @@ static void df_v3_6_pmc_get_count(struct
> amdgpu_device *adev,
> 
>  	switch (adev->asic_type) {
>  	case CHIP_VEGA20:
> +	case CHIP_ARCTURUS:
>  		df_v3_6_pmc_get_read_settings(adev, config, counter_idx,
>  						&lo_base_addr,
> &hi_base_addr);
> 
> --
> 2.17.1
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 2/3] drm/amdgpu: add per device user friendly xgmi events for vega20

[Jonathan Kim]

Non-outbound data metrics are non useful so mark them as legacy.
Bucket new perf counters into device and not device ip.
Bind events to chip instead of IP.
Report available event counters and not number of hw counter banks.
Move DF public macros to private since not needed outside of IP version.

v5: cleanup by moving per chip configs into structs

v4: After more discussion, replace *_LEGACY references with IP references
to indicate concept of pmu-typed versus event-config-typed event
registration.

v3: attr groups const array is global but attr groups are allocated per
device which doesn't work and causes problems on memory allocation and
de-allocation for pmu unregister. Switch to building const attr groups
per pmu instead to simplify solution.

v2: add comments on sysfs structure and formatting.

Signed-off-by: Jonathan Kim <jonathan.kim@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h     |  13 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c | 432 ++++++++++++++++++++----
 drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.h |  26 +-
 drivers/gpu/drm/amd/amdgpu/df_v3_6.c    |  72 +---
 drivers/gpu/drm/amd/amdgpu/df_v3_6.h    |   9 -
 5 files changed, 408 insertions(+), 144 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 13f92dea182a..f43dfdd2716a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1279,19 +1279,6 @@ bool amdgpu_device_load_pci_state(struct pci_dev *pdev);
 
 #include "amdgpu_object.h"
 
-/* used by df_v3_6.c and amdgpu_pmu.c */
-#define AMDGPU_PMU_ATTR(_name, _object)					\
-static ssize_t								\
-_name##_show(struct device *dev,					\
-			       struct device_attribute *attr,		\
-			       char *page)				\
-{									\
-	BUILD_BUG_ON(sizeof(_object) >= PAGE_SIZE - 1);			\
-	return sprintf(page, _object "\n");				\
-}									\
-									\
-static struct device_attribute pmu_attr_##_name = __ATTR_RO(_name)
-
 static inline bool amdgpu_is_tmz(struct amdgpu_device *adev)
 {
        return adev->gmc.tmz_enabled;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
index 1b0ec715c8ba..20c500f61b40 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
@@ -27,9 +27,20 @@
 #include <linux/init.h>
 #include "amdgpu.h"
 #include "amdgpu_pmu.h"
-#include "df_v3_6.h"
 
 #define PMU_NAME_SIZE 32
+#define NUM_FORMATS_AMDGPU_PMU		4
+#define NUM_FORMATS_DF_VEGA20		3
+#define NUM_EVENTS_DF_VEGA20		8
+#define NUM_EVENT_TYPES_VEGA20		1
+#define NUM_EVENTS_VEGA20_XGMI		2
+#define NUM_EVENTS_VEGA20_MAX		NUM_EVENTS_VEGA20_XGMI
+
+struct amdgpu_pmu_event_attribute {
+	struct device_attribute attr;
+	const char *event_str;
+	unsigned int type;
+};
 
 /* record to keep track of pmu entry per pmu type per device */
 struct amdgpu_pmu_entry {
@@ -37,11 +48,132 @@ struct amdgpu_pmu_entry {
 	struct amdgpu_device *adev;
 	struct pmu pmu;
 	unsigned int pmu_perf_type;
+	char *pmu_type_name;
+	char *pmu_file_prefix;
+	struct attribute_group fmt_attr_group;
+	struct amdgpu_pmu_event_attribute *fmt_attr;
+	struct attribute_group evt_attr_group;
+	struct amdgpu_pmu_event_attribute *evt_attr;
 };
 
+static ssize_t amdgpu_pmu_event_show(struct device *dev,
+				struct device_attribute *attr, char *buf)
+{
+	struct amdgpu_pmu_event_attribute *amdgpu_pmu_attr;
+
+	amdgpu_pmu_attr = container_of(attr, struct amdgpu_pmu_event_attribute,
+									attr);
+
+	if (!amdgpu_pmu_attr->type)
+		return sprintf(buf, "%s\n", amdgpu_pmu_attr->event_str);
+
+	return sprintf(buf, "%s,type=0x%x\n",
+			amdgpu_pmu_attr->event_str, amdgpu_pmu_attr->type);
+}
+
 static LIST_HEAD(amdgpu_pmu_list);
 
 
+struct amdgpu_pmu_attr {
+	const char *name;
+	const char *config;
+};
+
+struct amdgpu_pmu_type {
+	const unsigned int type;
+	const unsigned int num_of_type;
+};
+
+struct amdgpu_pmu_config {
+	struct amdgpu_pmu_attr *formats;
+	unsigned int num_formats;
+	struct amdgpu_pmu_attr *events;
+	unsigned int num_events;
+	struct amdgpu_pmu_type *types;
+	unsigned int num_types;
+};
+
+/*
+ * Events fall under two categories:
+ *  - PMU typed
+ *    Events in /sys/bus/event_source/devices/amdgpu_<pmu_type>_<dev_num> have
+ *    performance counter operations handled by one IP <pmu_type>.  Formats and
+ *    events should be defined by <pmu_type>_<asic_type>_formats and
+ *    <pmu_type>_<asic_type>_events respectively.
+ *
+ *  - Event config typed
+ *    Events in /sys/bus/event_source/devices/amdgpu_<dev_num> have performance
+ *    counter operations that can be handled by multiple IPs dictated by their
+ *    "type" format field.  Formats and events should be defined by
+ *    amdgpu_pmu_formats and <asic_type>_events respectively.  Format field
+ *    "type" is generated in amdgpu_pmu_event_show and defined in
+ *    <asic_type>_event_config_types.
+ */
+
+static struct amdgpu_pmu_attr amdgpu_pmu_formats[NUM_FORMATS_AMDGPU_PMU] = {
+	{ .name = "event", .config = "config:0-7" },
+	{ .name = "instance", .config = "config:8-15" },
+	{ .name = "umask", .config = "config:16-23"},
+	{ .name = "type", .config = "config:56-63"}
+};
+
+/* Vega20 events */
+static struct amdgpu_pmu_attr vega20_events[NUM_EVENTS_VEGA20_MAX] = {
+	{ .name = "xgmi_link0_data_outbound",
+			.config = "event=0x7,instance=0x46,umask=0x2" },
+	{ .name = "xgmi_link1_data_outbound",
+			.config = "event=0x7,instance=0x47,umask=0x2" }
+};
+
+static struct amdgpu_pmu_type vega20_types[NUM_EVENT_TYPES_VEGA20] = {
+	{ .type = AMDGPU_PMU_EVENT_CONFIG_TYPE_XGMI,
+					.num_of_type = NUM_EVENTS_VEGA20_XGMI }
+};
+
+static struct amdgpu_pmu_config vega20_config = {
+	.formats = amdgpu_pmu_formats,
+	.num_formats = ARRAY_SIZE(amdgpu_pmu_formats),
+	.events = vega20_events,
+	.num_events = ARRAY_SIZE(vega20_events),
+	.types = vega20_types,
+	.num_types = ARRAY_SIZE(vega20_types)
+};
+
+/* Vega20 data fabric (DF) events */
+static struct amdgpu_pmu_attr df_vega20_formats[NUM_FORMATS_DF_VEGA20] = {
+	{ .name = "event", .config = "config:0-7" },
+	{ .name = "instance", .config = "config:8-15" },
+	{ .name = "umask", .config = "config:16-23"}
+};
+
+static struct amdgpu_pmu_attr df_vega20_events[NUM_EVENTS_DF_VEGA20] = {
+	{ .name = "cake0_pcsout_txdata",
+			.config = "event=0x7,instance=0x46,umask=0x2" },
+	{ .name = "cake1_pcsout_txdata",
+			.config = "event=0x7,instance=0x47,umask=0x2" },
+	{ .name = "cake0_pcsout_txmeta",
+			.config = "event=0x7,instance=0x46,umask=0x4" },
+	{ .name = "cake1_pcsout_txmeta",
+			.config = "event=0x7,instance=0x47,umask=0x4" },
+	{ .name = "cake0_ftiinstat_reqalloc",
+			.config = "event=0xb,instance=0x46,umask=0x4" },
+	{ .name = "cake1_ftiinstat_reqalloc",
+			.config = "event=0xb,instance=0x47,umask=0x4" },
+	{ .name = "cake0_ftiinstat_rspalloc",
+			.config = "event=0xb,instance=0x46,umask=0x8" },
+	{ .name = "cake1_ftiinstat_rspalloc",
+			.config = "event=0xb,instance=0x47,umask=0x8" }
+};
+
+static struct amdgpu_pmu_config df_vega20_config = {
+	.formats = df_vega20_formats,
+	.num_formats = ARRAY_SIZE(df_vega20_formats),
+	.events = df_vega20_events,
+	.num_events = ARRAY_SIZE(df_vega20_events),
+	.types = NULL,
+	.num_types = 0
+};
+
 /* initialize perf counter */
 static int amdgpu_perf_event_init(struct perf_event *event)
 {
@@ -53,6 +185,7 @@ static int amdgpu_perf_event_init(struct perf_event *event)
 
 	/* update the hw_perf_event struct with config data */
 	hwc->config = event->attr.config;
+	hwc->config_base = AMDGPU_PMU_PERF_TYPE_NONE;
 
 	return 0;
 }
@@ -72,8 +205,9 @@ static void amdgpu_perf_start(struct perf_event *event, int flags)
 	WARN_ON_ONCE(!(hwc->state & PERF_HES_UPTODATE));
 	hwc->state = 0;
 
-	switch (pe->pmu_perf_type) {
-	case PERF_TYPE_AMDGPU_DF:
+	switch (hwc->config_base) {
+	case AMDGPU_PMU_EVENT_CONFIG_TYPE_DF:
+	case AMDGPU_PMU_EVENT_CONFIG_TYPE_XGMI:
 		if (!(flags & PERF_EF_RELOAD)) {
 			target_cntr = pe->adev->df.funcs->pmc_start(pe->adev,
 						hwc->config, 0 /* unused */,
@@ -101,14 +235,14 @@ static void amdgpu_perf_read(struct perf_event *event)
 	struct amdgpu_pmu_entry *pe = container_of(event->pmu,
 						  struct amdgpu_pmu_entry,
 						  pmu);
-
 	u64 count, prev;
 
 	do {
 		prev = local64_read(&hwc->prev_count);
 
-		switch (pe->pmu_perf_type) {
-		case PERF_TYPE_AMDGPU_DF:
+		switch (hwc->config_base) {
+		case AMDGPU_PMU_EVENT_CONFIG_TYPE_DF:
+		case AMDGPU_PMU_EVENT_CONFIG_TYPE_XGMI:
 			pe->adev->df.funcs->pmc_get_count(pe->adev,
 						hwc->config, hwc->idx, &count);
 			break;
@@ -132,8 +266,9 @@ static void amdgpu_perf_stop(struct perf_event *event, int flags)
 	if (hwc->state & PERF_HES_UPTODATE)
 		return;
 
-	switch (pe->pmu_perf_type) {
-	case PERF_TYPE_AMDGPU_DF:
+	switch (hwc->config_base) {
+	case AMDGPU_PMU_EVENT_CONFIG_TYPE_DF:
+	case AMDGPU_PMU_EVENT_CONFIG_TYPE_XGMI:
 		pe->adev->df.funcs->pmc_stop(pe->adev, hwc->config, hwc->idx,
 									0);
 		break;
@@ -160,10 +295,22 @@ static int amdgpu_perf_add(struct perf_event *event, int flags)
 						  struct amdgpu_pmu_entry,
 						  pmu);
 
+	switch (pe->pmu_perf_type) {
+	case AMDGPU_PMU_PERF_TYPE_DF:
+		hwc->config_base = AMDGPU_PMU_EVENT_CONFIG_TYPE_DF;
+		break;
+	case AMDGPU_PMU_PERF_TYPE_ALL:
+		hwc->config_base = (hwc->config >>
+					AMDGPU_PMU_EVENT_CONFIG_TYPE_SHIFT) &
+					AMDGPU_PMU_EVENT_CONFIG_TYPE_MASK;
+		break;
+	}
+
 	event->hw.state = PERF_HES_UPTODATE | PERF_HES_STOPPED;
 
-	switch (pe->pmu_perf_type) {
-	case PERF_TYPE_AMDGPU_DF:
+	switch (hwc->config_base) {
+	case AMDGPU_PMU_EVENT_CONFIG_TYPE_DF:
+	case AMDGPU_PMU_EVENT_CONFIG_TYPE_XGMI:
 		target_cntr = pe->adev->df.funcs->pmc_start(pe->adev,
 						hwc->config, 0 /* unused */,
 						1 /* add counter */);
@@ -196,8 +343,9 @@ static void amdgpu_perf_del(struct perf_event *event, int flags)
 
 	amdgpu_perf_stop(event, PERF_EF_UPDATE);
 
-	switch (pe->pmu_perf_type) {
-	case PERF_TYPE_AMDGPU_DF:
+	switch (hwc->config_base) {
+	case AMDGPU_PMU_EVENT_CONFIG_TYPE_DF:
+	case AMDGPU_PMU_EVENT_CONFIG_TYPE_XGMI:
 		pe->adev->df.funcs->pmc_stop(pe->adev, hwc->config, hwc->idx,
 									1);
 		break;
@@ -208,25 +356,92 @@ static void amdgpu_perf_del(struct perf_event *event, int flags)
 	perf_event_update_userpage(event);
 }
 
-/* vega20 pmus */
+static void amdgpu_pmu_create_event_attrs_by_type(
+				struct attribute_group *attr_group,
+				struct amdgpu_pmu_event_attribute *pmu_attr,
+				struct amdgpu_pmu_attr events[],
+				int s_offset,
+				int e_offset,
+				unsigned int type)
+{
+	int i;
+
+	pmu_attr += s_offset;
+
+	for (i = s_offset; i < e_offset; i++) {
+		attr_group->attrs[i] = &pmu_attr->attr.attr;
+		sysfs_attr_init(&pmu_attr->attr.attr);
+		pmu_attr->attr.attr.name = events[i].name;
+		pmu_attr->attr.attr.mode = 0444;
+		pmu_attr->attr.show = amdgpu_pmu_event_show;
+		pmu_attr->event_str = events[i].config;
+		pmu_attr->type = type;
+		pmu_attr++;
+	}
+}
 
-/* init pmu tracking per pmu type */
-static int init_pmu_by_type(struct amdgpu_device *adev,
-		  const struct attribute_group *attr_groups[],
-		  char *pmu_type_name, char *pmu_file_prefix,
-		  unsigned int pmu_perf_type,
-		  unsigned int num_counters)
+static void amdgpu_pmu_create_attrs(struct attribute_group *attr_group,
+				struct amdgpu_pmu_event_attribute *pmu_attr,
+				struct amdgpu_pmu_attr events[],
+				int num_events)
 {
-	char pmu_name[PMU_NAME_SIZE];
-	struct amdgpu_pmu_entry *pmu_entry;
-	int ret = 0;
+	amdgpu_pmu_create_event_attrs_by_type(attr_group, pmu_attr, events, 0,
+				num_events, AMDGPU_PMU_EVENT_CONFIG_TYPE_NONE);
+}
 
-	pmu_entry = kzalloc(sizeof(struct amdgpu_pmu_entry), GFP_KERNEL);
 
-	if (!pmu_entry)
+static int amdgpu_pmu_alloc_pmu_attrs(
+				struct attribute_group *fmt_attr_group,
+				struct amdgpu_pmu_event_attribute **fmt_attr,
+				struct attribute_group *evt_attr_group,
+				struct amdgpu_pmu_event_attribute **evt_attr,
+				struct amdgpu_pmu_config *config)
+{
+	*fmt_attr = kcalloc(config->num_formats, sizeof(**fmt_attr),
+								GFP_KERNEL);
+
+	if (!(*fmt_attr))
 		return -ENOMEM;
 
-	pmu_entry->adev = adev;
+	fmt_attr_group->attrs = kcalloc(config->num_formats + 1,
+				sizeof(*fmt_attr_group->attrs), GFP_KERNEL);
+
+	if (!fmt_attr_group->attrs)
+		goto err_fmt_attr_grp;
+
+	*evt_attr = kcalloc(config->num_events, sizeof(**evt_attr), GFP_KERNEL);
+
+	if (!(*evt_attr))
+		goto err_evt_attr;
+
+	evt_attr_group->attrs = kcalloc(config->num_events + 1,
+				sizeof(*evt_attr_group->attrs), GFP_KERNEL);
+
+	if (!evt_attr_group->attrs)
+		goto err_evt_attr_grp;
+
+	return 0;
+err_evt_attr_grp:
+	kfree(*evt_attr);
+err_evt_attr:
+	kfree(fmt_attr_group->attrs);
+err_fmt_attr_grp:
+	kfree(*fmt_attr);
+	return -ENOMEM;
+}
+
+/* init pmu tracking per pmu type */
+static int init_pmu_entry_by_type_and_add(struct amdgpu_pmu_entry *pmu_entry,
+			struct amdgpu_pmu_config *config)
+{
+	const struct attribute_group *attr_groups[] = {
+		&pmu_entry->fmt_attr_group,
+		&pmu_entry->evt_attr_group,
+		NULL
+	};
+	char pmu_name[PMU_NAME_SIZE];
+	int ret = 0, total_num_events = 0;
+
 	pmu_entry->pmu = (struct pmu){
 		.event_init = amdgpu_perf_event_init,
 		.add = amdgpu_perf_add,
@@ -237,59 +452,162 @@ static int init_pmu_by_type(struct amdgpu_device *adev,
 		.task_ctx_nr = perf_invalid_context,
 	};
 
-	pmu_entry->pmu.attr_groups = attr_groups;
-	pmu_entry->pmu_perf_type = pmu_perf_type;
-	snprintf(pmu_name, PMU_NAME_SIZE, "%s_%d",
-				pmu_file_prefix, adev_to_drm(adev)->primary->index);
+	ret = amdgpu_pmu_alloc_pmu_attrs(&pmu_entry->fmt_attr_group,
+					&pmu_entry->fmt_attr,
+					&pmu_entry->evt_attr_group,
+					&pmu_entry->evt_attr,
+					config);
+
+	if (ret)
+		goto err_out;
+
+	amdgpu_pmu_create_attrs(&pmu_entry->fmt_attr_group, pmu_entry->fmt_attr,
+					config->formats, config->num_formats);
+
+	if (pmu_entry->pmu_perf_type == AMDGPU_PMU_PERF_TYPE_ALL) {
+		int i;
+
+		for (i = 0; i < config->num_types; i++) {
+			amdgpu_pmu_create_event_attrs_by_type(
+					&pmu_entry->evt_attr_group,
+					pmu_entry->evt_attr,
+					config->events,
+					total_num_events,
+					total_num_events +
+						config->types[i].num_of_type,
+					config->types[i].type);
+			total_num_events += config->types[i].num_of_type;
+		}
+	} else {
+		amdgpu_pmu_create_attrs(&pmu_entry->evt_attr_group,
+					pmu_entry->evt_attr,
+					config->events, config->num_events);
+		total_num_events = config->num_events;
+	}
+
+	pmu_entry->pmu.attr_groups = kmemdup(attr_groups, sizeof(attr_groups),
+								GFP_KERNEL);
+
+	if (!pmu_entry->pmu.attr_groups)
+		goto err_attr_group;
+
+	snprintf(pmu_name, PMU_NAME_SIZE, "%s_%d", pmu_entry->pmu_file_prefix,
+				adev_to_drm(pmu_entry->adev)->primary->index);
 
 	ret = perf_pmu_register(&pmu_entry->pmu, pmu_name, -1);
 
-	if (ret) {
-		kfree(pmu_entry);
-		pr_warn("Error initializing AMDGPU %s PMUs.\n", pmu_type_name);
-		return ret;
-	}
+	if (ret)
+		goto err_register;
+
+	if (pmu_entry->pmu_perf_type != AMDGPU_PMU_PERF_TYPE_ALL)
+		pr_info("Detected AMDGPU %s Counters. # of Counters = %d.\n",
+				pmu_entry->pmu_type_name, total_num_events);
+	else
+		pr_info("Detected AMDGPU %d Perf Events.\n", total_num_events);
 
-	pr_info("Detected AMDGPU %s Counters. # of Counters = %d.\n",
-			pmu_type_name, num_counters);
 
 	list_add_tail(&pmu_entry->entry, &amdgpu_pmu_list);
 
 	return 0;
+err_register:
+	kfree(pmu_entry->pmu.attr_groups);
+err_attr_group:
+	kfree(pmu_entry->fmt_attr_group.attrs);
+	kfree(pmu_entry->fmt_attr);
+	kfree(pmu_entry->evt_attr_group.attrs);
+	kfree(pmu_entry->evt_attr);
+err_out:
+	pr_warn("Error initializing AMDGPU %s PMUs.\n",
+						pmu_entry->pmu_type_name);
+	return ret;
+}
+
+/* destroy all pmu data associated with target device */
+void amdgpu_pmu_fini(struct amdgpu_device *adev)
+{
+	struct amdgpu_pmu_entry *pe, *temp;
+
+	list_for_each_entry_safe(pe, temp, &amdgpu_pmu_list, entry) {
+		if (pe->adev != adev)
+			continue;
+		list_del(&pe->entry);
+		perf_pmu_unregister(&pe->pmu);
+		kfree(pe->pmu.attr_groups);
+		kfree(pe->fmt_attr_group.attrs);
+		kfree(pe->fmt_attr);
+		kfree(pe->evt_attr_group.attrs);
+		kfree(pe->evt_attr);
+		kfree(pe);
+	}
+}
+
+static struct amdgpu_pmu_entry *create_pmu_entry(struct amdgpu_device *adev,
+						unsigned int pmu_type,
+						char *pmu_type_name,
+						char *pmu_file_prefix)
+{
+	struct amdgpu_pmu_entry *pmu_entry;
+
+	pmu_entry = kzalloc(sizeof(struct amdgpu_pmu_entry), GFP_KERNEL);
+
+	if (!pmu_entry)
+		return pmu_entry;
+
+	pmu_entry->adev = adev;
+	pmu_entry->fmt_attr_group.name = "format";
+	pmu_entry->fmt_attr_group.attrs = NULL;
+	pmu_entry->evt_attr_group.name = "events";
+	pmu_entry->evt_attr_group.attrs = NULL;
+	pmu_entry->pmu_perf_type = pmu_type;
+	pmu_entry->pmu_type_name = pmu_type_name;
+	pmu_entry->pmu_file_prefix = pmu_file_prefix;
+
+	return pmu_entry;
 }
 
 /* init amdgpu_pmu */
 int amdgpu_pmu_init(struct amdgpu_device *adev)
 {
 	int ret = 0;
+	struct amdgpu_pmu_entry *pmu_entry, *pmu_entry_df;
 
 	switch (adev->asic_type) {
 	case CHIP_VEGA20:
-		/* init df */
-		ret = init_pmu_by_type(adev, df_v3_6_attr_groups,
-				       "DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF,
-				       DF_V3_6_MAX_COUNTERS);
+		pmu_entry_df = create_pmu_entry(adev, AMDGPU_PMU_PERF_TYPE_DF,
+						"DF", "amdgpu_df");
 
-		/* other pmu types go here*/
-		break;
-	default:
-		return 0;
-	}
+		if (!pmu_entry_df)
+			return -ENOMEM;
 
-	return 0;
-}
+		ret = init_pmu_entry_by_type_and_add(pmu_entry_df,
+							&df_vega20_config);
 
+		if (ret) {
+			kfree(pmu_entry_df);
+			return ret;
+		}
 
-/* destroy all pmu data associated with target device */
-void amdgpu_pmu_fini(struct amdgpu_device *adev)
-{
-	struct amdgpu_pmu_entry *pe, *temp;
+		pmu_entry = create_pmu_entry(adev, AMDGPU_PMU_PERF_TYPE_ALL,
+						"", "amdgpu");
 
-	list_for_each_entry_safe(pe, temp, &amdgpu_pmu_list, entry) {
-		if (pe->adev == adev) {
-			list_del(&pe->entry);
-			perf_pmu_unregister(&pe->pmu);
-			kfree(pe);
+		if (!pmu_entry) {
+			amdgpu_pmu_fini(adev);
+			return -ENOMEM;
 		}
-	}
+
+		ret = init_pmu_entry_by_type_and_add(pmu_entry,
+							&vega20_config);
+
+		if (ret) {
+			kfree(pmu_entry);
+			amdgpu_pmu_fini(adev);
+			return ret;
+		}
+
+		break;
+	default:
+		return 0;
+	};
+
+	return ret;
 }
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.h
index 7dddb7160a11..08b9f7ca6375 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.h
@@ -26,11 +26,33 @@
 #ifndef _AMDGPU_PMU_H_
 #define _AMDGPU_PMU_H_
 
+/* PMU types. */
 enum amdgpu_pmu_perf_type {
-	PERF_TYPE_AMDGPU_DF = 0,
-	PERF_TYPE_AMDGPU_MAX
+	AMDGPU_PMU_PERF_TYPE_NONE = 0,
+	AMDGPU_PMU_PERF_TYPE_DF,
+	AMDGPU_PMU_PERF_TYPE_ALL
 };
 
+/*
+ * PMU type AMDGPU_PMU_PERF_TYPE_ALL can hold events of different "type"
+ * configurations.  Event config types are parsed from the 64-bit raw
+ * config (See EVENT_CONFIG_TYPE_SHIFT and EVENT_CONFIG_TYPE_MASK) and
+ * are registered into the HW perf events config_base.
+ *
+ * PMU types with only a single event configuration type
+ * (non-AMDGPU_PMU_PERF_TYPE_ALL) have their event config type auto generated
+ * when the performance counter is added.
+ */
+enum amdgpu_pmu_event_config_type {
+	AMDGPU_PMU_EVENT_CONFIG_TYPE_NONE = 0,
+	AMDGPU_PMU_EVENT_CONFIG_TYPE_DF,
+	AMDGPU_PMU_EVENT_CONFIG_TYPE_XGMI,
+	AMDGPU_PMU_EVENT_CONFIG_TYPE_MAX
+};
+
+#define AMDGPU_PMU_EVENT_CONFIG_TYPE_SHIFT	56
+#define AMDGPU_PMU_EVENT_CONFIG_TYPE_MASK	0xff
+
 int amdgpu_pmu_init(struct amdgpu_device *adev);
 void amdgpu_pmu_fini(struct amdgpu_device *adev);
 
diff --git a/drivers/gpu/drm/amd/amdgpu/df_v3_6.c b/drivers/gpu/drm/amd/amdgpu/df_v3_6.c
index 0ca6e176acb0..6e57ae95f997 100644
--- a/drivers/gpu/drm/amd/amdgpu/df_v3_6.c
+++ b/drivers/gpu/drm/amd/amdgpu/df_v3_6.c
@@ -30,71 +30,17 @@
 #define DF_3_6_SMN_REG_INST_DIST        0x8
 #define DF_3_6_INST_CNT                 8
 
-static u32 df_v3_6_channel_number[] = {1, 2, 0, 4, 0, 8, 0,
-				       16, 32, 0, 0, 0, 2, 4, 8};
-
-/* init df format attrs */
-AMDGPU_PMU_ATTR(event,		"config:0-7");
-AMDGPU_PMU_ATTR(instance,	"config:8-15");
-AMDGPU_PMU_ATTR(umask,		"config:16-23");
-
-/* df format attributes  */
-static struct attribute *df_v3_6_format_attrs[] = {
-	&pmu_attr_event.attr,
-	&pmu_attr_instance.attr,
-	&pmu_attr_umask.attr,
-	NULL
-};
-
-/* df format attribute group */
-static struct attribute_group df_v3_6_format_attr_group = {
-	.name = "format",
-	.attrs = df_v3_6_format_attrs,
-};
+/* Defined in global_features.h as FTI_PERFMON_VISIBLE */
+#define DF_V3_6_MAX_COUNTERS		4
 
-/* df event attrs */
-AMDGPU_PMU_ATTR(cake0_pcsout_txdata,
-		      "event=0x7,instance=0x46,umask=0x2");
-AMDGPU_PMU_ATTR(cake1_pcsout_txdata,
-		      "event=0x7,instance=0x47,umask=0x2");
-AMDGPU_PMU_ATTR(cake0_pcsout_txmeta,
-		      "event=0x7,instance=0x46,umask=0x4");
-AMDGPU_PMU_ATTR(cake1_pcsout_txmeta,
-		      "event=0x7,instance=0x47,umask=0x4");
-AMDGPU_PMU_ATTR(cake0_ftiinstat_reqalloc,
-		      "event=0xb,instance=0x46,umask=0x4");
-AMDGPU_PMU_ATTR(cake1_ftiinstat_reqalloc,
-		      "event=0xb,instance=0x47,umask=0x4");
-AMDGPU_PMU_ATTR(cake0_ftiinstat_rspalloc,
-		      "event=0xb,instance=0x46,umask=0x8");
-AMDGPU_PMU_ATTR(cake1_ftiinstat_rspalloc,
-		      "event=0xb,instance=0x47,umask=0x8");
-
-/* df event attributes  */
-static struct attribute *df_v3_6_event_attrs[] = {
-	&pmu_attr_cake0_pcsout_txdata.attr,
-	&pmu_attr_cake1_pcsout_txdata.attr,
-	&pmu_attr_cake0_pcsout_txmeta.attr,
-	&pmu_attr_cake1_pcsout_txmeta.attr,
-	&pmu_attr_cake0_ftiinstat_reqalloc.attr,
-	&pmu_attr_cake1_ftiinstat_reqalloc.attr,
-	&pmu_attr_cake0_ftiinstat_rspalloc.attr,
-	&pmu_attr_cake1_ftiinstat_rspalloc.attr,
-	NULL
-};
-
-/* df event attribute group */
-static struct attribute_group df_v3_6_event_attr_group = {
-	.name = "events",
-	.attrs = df_v3_6_event_attrs
-};
+/* get flags from df perfmon config */
+#define DF_V3_6_GET_EVENT(x)		(x & 0xFFUL)
+#define DF_V3_6_GET_INSTANCE(x)		((x >> 8) & 0xFFUL)
+#define DF_V3_6_GET_UNITMASK(x)		((x >> 16) & 0xFFUL)
+#define DF_V3_6_PERFMON_OVERFLOW	0xFFFFFFFFFFFFULL
 
-/* df event attr groups  */
-const struct attribute_group *df_v3_6_attr_groups[] = {
-		&df_v3_6_format_attr_group,
-		&df_v3_6_event_attr_group,
-		NULL
-};
+static u32 df_v3_6_channel_number[] = {1, 2, 0, 4, 0, 8, 0,
+				       16, 32, 0, 0, 0, 2, 4, 8};
 
 static uint64_t df_v3_6_get_fica(struct amdgpu_device *adev,
 				 uint32_t ficaa_val)
diff --git a/drivers/gpu/drm/amd/amdgpu/df_v3_6.h b/drivers/gpu/drm/amd/amdgpu/df_v3_6.h
index 76998541bc30..2505c7ef258a 100644
--- a/drivers/gpu/drm/amd/amdgpu/df_v3_6.h
+++ b/drivers/gpu/drm/amd/amdgpu/df_v3_6.h
@@ -35,15 +35,6 @@ enum DF_V3_6_MGCG {
 	DF_V3_6_MGCG_ENABLE_63_CYCLE_DELAY = 15
 };
 
-/* Defined in global_features.h as FTI_PERFMON_VISIBLE */
-#define DF_V3_6_MAX_COUNTERS		4
-
-/* get flags from df perfmon config */
-#define DF_V3_6_GET_EVENT(x)		(x & 0xFFUL)
-#define DF_V3_6_GET_INSTANCE(x)		((x >> 8) & 0xFFUL)
-#define DF_V3_6_GET_UNITMASK(x)		((x >> 16) & 0xFFUL)
-#define DF_V3_6_PERFMON_OVERFLOW	0xFFFFFFFFFFFFULL
-
 extern const struct attribute_group *df_v3_6_attr_groups[];
 extern const struct amdgpu_df_funcs df_v3_6_funcs;
 
-- 
2.17.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 2/3] drm/amdgpu: add per device user friendly xgmi events for vega20

[Jonathan Kim]

Non-outbound data metrics are non useful so mark them as legacy.
Bucket new perf counters into device and not device ip.
Bind events to chip instead of IP.
Report available event counters and not number of hw counter banks.
Move DF public macros to private since not needed outside of IP version.

v4: After more discussion, abandon the idea of legacy events and instead
use the concept of pmu-typed versus event-config-typed events for
flexibility.

v3: attr groups const array is global but attr groups are allocated per
device which doesn't work and causes problems on memory allocation and
de-allocation for pmu unregister. Switch to building const attr groups
per pmu instead to simplify solution.

v2: add comments on sysfs structure and formatting.

Signed-off-by: Jonathan Kim <Jonathan.Kim@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h     |  13 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c | 380 ++++++++++++++++++++----
 drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.h |  26 +-
 drivers/gpu/drm/amd/amdgpu/df_v3_6.c    |  72 +----
 drivers/gpu/drm/amd/amdgpu/df_v3_6.h    |   9 -
 5 files changed, 356 insertions(+), 144 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 13f92dea182a..f43dfdd2716a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1279,19 +1279,6 @@ bool amdgpu_device_load_pci_state(struct pci_dev *pdev);
 
 #include "amdgpu_object.h"
 
-/* used by df_v3_6.c and amdgpu_pmu.c */
-#define AMDGPU_PMU_ATTR(_name, _object)					\
-static ssize_t								\
-_name##_show(struct device *dev,					\
-			       struct device_attribute *attr,		\
-			       char *page)				\
-{									\
-	BUILD_BUG_ON(sizeof(_object) >= PAGE_SIZE - 1);			\
-	return sprintf(page, _object "\n");				\
-}									\
-									\
-static struct device_attribute pmu_attr_##_name = __ATTR_RO(_name)
-
 static inline bool amdgpu_is_tmz(struct amdgpu_device *adev)
 {
        return adev->gmc.tmz_enabled;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
index 1b0ec715c8ba..610f96bb0239 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
@@ -27,9 +27,20 @@
 #include <linux/init.h>
 #include "amdgpu.h"
 #include "amdgpu_pmu.h"
-#include "df_v3_6.h"
 
 #define PMU_NAME_SIZE 32
+#define NUM_FORMATS_AMDGPU_PMU		4
+#define NUM_FORMATS_DF_VEGA20		3
+#define NUM_EVENTS_DF_VEGA20		8
+#define NUM_EVENT_TYPES_VEGA20		1
+#define NUM_EVENTS_VEGA20_XGMI		2
+#define NUM_EVENTS_VEGA20_MAX		NUM_EVENTS_VEGA20_XGMI
+
+struct amdgpu_pmu_event_attribute {
+	struct device_attribute attr;
+	const char *event_str;
+	unsigned int type;
+};
 
 /* record to keep track of pmu entry per pmu type per device */
 struct amdgpu_pmu_entry {
@@ -37,10 +48,79 @@ struct amdgpu_pmu_entry {
 	struct amdgpu_device *adev;
 	struct pmu pmu;
 	unsigned int pmu_perf_type;
+	struct attribute_group fmt_attr_group;
+	struct amdgpu_pmu_event_attribute *fmt_attr;
+	struct attribute_group evt_attr_group;
+	struct amdgpu_pmu_event_attribute *evt_attr;
 };
 
+static ssize_t amdgpu_pmu_event_show(struct device *dev,
+				struct device_attribute *attr, char *buf)
+{
+	struct amdgpu_pmu_event_attribute *amdgpu_pmu_attr;
+
+	amdgpu_pmu_attr = container_of(attr, struct amdgpu_pmu_event_attribute,
+									attr);
+
+	if (!amdgpu_pmu_attr->type)
+		return sprintf(buf, "%s\n", amdgpu_pmu_attr->event_str);
+
+	return sprintf(buf, "%s,type=0x%x\n",
+			amdgpu_pmu_attr->event_str, amdgpu_pmu_attr->type);
+}
+
 static LIST_HEAD(amdgpu_pmu_list);
 
+/*
+ * Events fall under two categories:
+ *  - PMU typed
+ *    Events in /sys/bus/event_source/devices/amdgpu_<pmu_type>_<dev_num> have
+ *    performance counter operations handled by one IP <pmu_type>.  Formats and
+ *    events should be defined by <pmu_type>_<asic_type>_formats and
+ *    <pmu_type>_<asic_type>_events respectively.
+ *
+ *  - Event config typed
+ *    Events in /sys/bus/event_source/devices/amdgpu_<dev_num> have performance
+ *    counter operations that can be handled by multiple IPs dictated by their
+ *    "type" format field.  Formats and events should be defined by
+ *    amdgpu_pmu_formats and <asic_type>_events respectively.  Format field
+ *    "type" is generated in amdgpu_pmu_event_show and defined in
+ *    <asic_type>_event_config_types.
+ */
+static const char *amdgpu_pmu_formats[NUM_FORMATS_AMDGPU_PMU][2] = {
+	{ "event", "config:0-7" },
+	{ "instance", "config:8-15" },
+	{ "umask", "config:16-23"},
+	{ "type", "config:56-63"}
+};
+
+/* Vega20 events */
+static const char *vega20_events[NUM_EVENTS_VEGA20_MAX][2] = {
+	{ "xgmi_link0_data_outbound", "event=0x7,instance=0x46,umask=0x2" },
+	{ "xgmi_link1_data_outbound", "event=0x7,instance=0x47,umask=0x2" }
+};
+
+static const int vega20_event_config_types[NUM_EVENT_TYPES_VEGA20][2] = {
+	{ AMDGPU_PMU_EVENT_CONFIG_TYPE_XGMI, NUM_EVENTS_VEGA20_XGMI }
+};
+
+/* Vega20 DF formats and events */
+static const char *df_vega20_formats[NUM_FORMATS_DF_VEGA20][2] = {
+	{ "event", "config:0-7" },
+	{ "instance", "config:8-15" },
+	{ "umask", "config:16-23"},
+};
+
+static const char *df_vega20_events[NUM_EVENTS_DF_VEGA20][2] = {
+	{ "cake0_pcsout_txdata", "event=0x7,instance=0x46,umask=0x2" },
+	{ "cake1_pcsout_txdata", "event=0x7,instance=0x47,umask=0x2" },
+	{ "cake0_pcsout_txmeta", "event=0x7,instance=0x46,umask=0x4" },
+	{ "cake1_pcsout_txmeta", "event=0x7,instance=0x47,umask=0x4" },
+	{ "cake0_ftiinstat_reqalloc", "event=0xb,instance=0x46,umask=0x4" },
+	{ "cake1_ftiinstat_reqalloc", "event=0xb,instance=0x47,umask=0x4" },
+	{ "cake0_ftiinstat_rspalloc", "event=0xb,instance=0x46,umask=0x8" },
+	{ "cake1_ftiinstat_rspalloc", "event=0xb,instance=0x47,umask=0x8" },
+};
 
 /* initialize perf counter */
 static int amdgpu_perf_event_init(struct perf_event *event)
@@ -53,6 +133,7 @@ static int amdgpu_perf_event_init(struct perf_event *event)
 
 	/* update the hw_perf_event struct with config data */
 	hwc->config = event->attr.config;
+	hwc->config_base = AMDGPU_PMU_PERF_TYPE_NONE;
 
 	return 0;
 }
@@ -72,8 +153,9 @@ static void amdgpu_perf_start(struct perf_event *event, int flags)
 	WARN_ON_ONCE(!(hwc->state & PERF_HES_UPTODATE));
 	hwc->state = 0;
 
-	switch (pe->pmu_perf_type) {
-	case PERF_TYPE_AMDGPU_DF:
+	switch (hwc->config_base) {
+	case AMDGPU_PMU_EVENT_CONFIG_TYPE_DF:
+	case AMDGPU_PMU_EVENT_CONFIG_TYPE_XGMI:
 		if (!(flags & PERF_EF_RELOAD)) {
 			target_cntr = pe->adev->df.funcs->pmc_start(pe->adev,
 						hwc->config, 0 /* unused */,
@@ -101,14 +183,14 @@ static void amdgpu_perf_read(struct perf_event *event)
 	struct amdgpu_pmu_entry *pe = container_of(event->pmu,
 						  struct amdgpu_pmu_entry,
 						  pmu);
-
 	u64 count, prev;
 
 	do {
 		prev = local64_read(&hwc->prev_count);
 
-		switch (pe->pmu_perf_type) {
-		case PERF_TYPE_AMDGPU_DF:
+		switch (hwc->config_base) {
+		case AMDGPU_PMU_EVENT_CONFIG_TYPE_DF:
+		case AMDGPU_PMU_EVENT_CONFIG_TYPE_XGMI:
 			pe->adev->df.funcs->pmc_get_count(pe->adev,
 						hwc->config, hwc->idx, &count);
 			break;
@@ -132,8 +214,9 @@ static void amdgpu_perf_stop(struct perf_event *event, int flags)
 	if (hwc->state & PERF_HES_UPTODATE)
 		return;
 
-	switch (pe->pmu_perf_type) {
-	case PERF_TYPE_AMDGPU_DF:
+	switch (hwc->config_base) {
+	case AMDGPU_PMU_EVENT_CONFIG_TYPE_DF:
+	case AMDGPU_PMU_EVENT_CONFIG_TYPE_XGMI:
 		pe->adev->df.funcs->pmc_stop(pe->adev, hwc->config, hwc->idx,
 									0);
 		break;
@@ -160,10 +243,22 @@ static int amdgpu_perf_add(struct perf_event *event, int flags)
 						  struct amdgpu_pmu_entry,
 						  pmu);
 
+	switch (pe->pmu_perf_type) {
+	case AMDGPU_PMU_PERF_TYPE_DF:
+		hwc->config_base = AMDGPU_PMU_EVENT_CONFIG_TYPE_DF;
+		break;
+	case AMDGPU_PMU_PERF_TYPE_ALL:
+		hwc->config_base = (hwc->config >>
+					AMDGPU_PMU_EVENT_CONFIG_TYPE_SHIFT) &
+					AMDGPU_PMU_EVENT_CONFIG_TYPE_MASK;
+		break;
+	}
+
 	event->hw.state = PERF_HES_UPTODATE | PERF_HES_STOPPED;
 
-	switch (pe->pmu_perf_type) {
-	case PERF_TYPE_AMDGPU_DF:
+	switch (hwc->config_base) {
+	case AMDGPU_PMU_EVENT_CONFIG_TYPE_DF:
+	case AMDGPU_PMU_EVENT_CONFIG_TYPE_XGMI:
 		target_cntr = pe->adev->df.funcs->pmc_start(pe->adev,
 						hwc->config, 0 /* unused */,
 						1 /* add counter */);
@@ -196,8 +291,9 @@ static void amdgpu_perf_del(struct perf_event *event, int flags)
 
 	amdgpu_perf_stop(event, PERF_EF_UPDATE);
 
-	switch (pe->pmu_perf_type) {
-	case PERF_TYPE_AMDGPU_DF:
+	switch (hwc->config_base) {
+	case AMDGPU_PMU_EVENT_CONFIG_TYPE_DF:
+	case AMDGPU_PMU_EVENT_CONFIG_TYPE_XGMI:
 		pe->adev->df.funcs->pmc_stop(pe->adev, hwc->config, hwc->idx,
 									1);
 		break;
@@ -208,25 +304,96 @@ static void amdgpu_perf_del(struct perf_event *event, int flags)
 	perf_event_update_userpage(event);
 }
 
-/* vega20 pmus */
+static void amdgpu_pmu_create_event_attrs_by_type(
+				struct attribute_group *attr_group,
+				struct amdgpu_pmu_event_attribute *pmu_attr,
+				const char *events[][2],
+				int s_offset,
+				int e_offset,
+				unsigned int type)
+{
+	int i;
+
+	pmu_attr += s_offset;
+
+	for (i = s_offset; i < e_offset; i++) {
+		attr_group->attrs[i] = &pmu_attr->attr.attr;
+		sysfs_attr_init(&pmu_attr->attr.attr);
+		pmu_attr->attr.attr.name = events[i][0];
+		pmu_attr->attr.attr.mode = 0444;
+		pmu_attr->attr.show = amdgpu_pmu_event_show;
+		pmu_attr->event_str = events[i][1];
+		pmu_attr->type = type;
+		pmu_attr++;
+	}
+}
 
-/* init pmu tracking per pmu type */
-static int init_pmu_by_type(struct amdgpu_device *adev,
-		  const struct attribute_group *attr_groups[],
-		  char *pmu_type_name, char *pmu_file_prefix,
-		  unsigned int pmu_perf_type,
-		  unsigned int num_counters)
+static void amdgpu_pmu_create_attrs(struct attribute_group *attr_group,
+				struct amdgpu_pmu_event_attribute *pmu_attr,
+				const char *events[][2], int num_events)
 {
-	char pmu_name[PMU_NAME_SIZE];
-	struct amdgpu_pmu_entry *pmu_entry;
-	int ret = 0;
+	amdgpu_pmu_create_event_attrs_by_type(attr_group, pmu_attr, events, 0,
+			num_events, AMDGPU_PMU_EVENT_CONFIG_TYPE_NONE);
+}
 
-	pmu_entry = kzalloc(sizeof(struct amdgpu_pmu_entry), GFP_KERNEL);
 
-	if (!pmu_entry)
+static int amdgpu_pmu_alloc_pmu_attrs(
+				struct attribute_group *fmt_attr_group,
+				struct amdgpu_pmu_event_attribute **fmt_attr,
+				int fmt_num_attrs,
+				struct attribute_group *evt_attr_group,
+				struct amdgpu_pmu_event_attribute **evt_attr,
+				int evt_num_attrs)
+{
+	*fmt_attr = kcalloc(fmt_num_attrs, sizeof(**fmt_attr), GFP_KERNEL);
+
+	if (!(*fmt_attr))
 		return -ENOMEM;
 
-	pmu_entry->adev = adev;
+	fmt_attr_group->attrs = kcalloc(fmt_num_attrs + 1,
+				sizeof(*fmt_attr_group->attrs), GFP_KERNEL);
+
+	if (!fmt_attr_group->attrs)
+		goto err_fmt_attr_grp;
+
+	*evt_attr = kcalloc(evt_num_attrs, sizeof(**evt_attr), GFP_KERNEL);
+
+	if (!(*evt_attr))
+		goto err_evt_attr;
+
+	evt_attr_group->attrs = kcalloc(evt_num_attrs + 1,
+				sizeof(*evt_attr_group->attrs), GFP_KERNEL);
+
+	if (!evt_attr_group->attrs)
+		goto err_evt_attr_grp;
+
+	return 0;
+err_evt_attr_grp:
+	kfree(*evt_attr);
+err_evt_attr:
+	kfree(fmt_attr_group->attrs);
+err_fmt_attr_grp:
+	kfree(*fmt_attr);
+	return -ENOMEM;
+}
+
+/* init pmu tracking per pmu type */
+static int init_pmu_by_type(struct amdgpu_pmu_entry *pmu_entry,
+			char *pmu_type_name, char *pmu_file_prefix,
+			unsigned int pmu_perf_type,
+			const char *formats[][2], int num_formats,
+			const char *events[][2], int num_events,
+			const int event_config_types[256][2],
+			int num_config_types)
+{
+	const struct attribute_group *attr_groups[] = {
+		&pmu_entry->fmt_attr_group,
+		&pmu_entry->evt_attr_group,
+		NULL
+	};
+	char pmu_name[PMU_NAME_SIZE];
+	int ret = 0, total_num_events = 0;
+
 	pmu_entry->pmu = (struct pmu){
 		.event_init = amdgpu_perf_event_init,
 		.add = amdgpu_perf_add,
@@ -237,59 +404,158 @@ static int init_pmu_by_type(struct amdgpu_device *adev,
 		.task_ctx_nr = perf_invalid_context,
 	};
 
-	pmu_entry->pmu.attr_groups = attr_groups;
+	ret = amdgpu_pmu_alloc_pmu_attrs(&pmu_entry->fmt_attr_group,
+					&pmu_entry->fmt_attr,
+					num_formats,
+					&pmu_entry->evt_attr_group,
+					&pmu_entry->evt_attr,
+					num_events);
+
+	if (ret)
+		goto err_out;
+
+	amdgpu_pmu_create_attrs(&pmu_entry->fmt_attr_group, pmu_entry->fmt_attr,
+							formats, num_formats);
+
+	if (pmu_perf_type == AMDGPU_PMU_PERF_TYPE_ALL) {
+		int i;
+
+		for (i = 0; i < num_config_types; i++) {
+			amdgpu_pmu_create_event_attrs_by_type(
+						&pmu_entry->evt_attr_group,
+						pmu_entry->evt_attr,
+						events,
+						total_num_events,
+						event_config_types[i][1],
+						event_config_types[i][0]);
+			total_num_events += event_config_types[i][1];
+		}
+	} else {
+		amdgpu_pmu_create_attrs(&pmu_entry->evt_attr_group,
+					pmu_entry->evt_attr,
+					events, num_events);
+		total_num_events = num_events;
+	}
+
+	pmu_entry->pmu.attr_groups = kmemdup(attr_groups, sizeof(attr_groups),
+								GFP_KERNEL);
+
+	if (!pmu_entry->pmu.attr_groups)
+		goto err_attr_group;
+
 	pmu_entry->pmu_perf_type = pmu_perf_type;
-	snprintf(pmu_name, PMU_NAME_SIZE, "%s_%d",
-				pmu_file_prefix, adev_to_drm(adev)->primary->index);
+	snprintf(pmu_name, PMU_NAME_SIZE, "%s_%d", pmu_file_prefix,
+				adev_to_drm(pmu_entry->adev)->primary->index);
 
 	ret = perf_pmu_register(&pmu_entry->pmu, pmu_name, -1);
 
-	if (ret) {
-		kfree(pmu_entry);
-		pr_warn("Error initializing AMDGPU %s PMUs.\n", pmu_type_name);
-		return ret;
-	}
+	if (ret)
+		goto err_register;
+
+	if (pmu_perf_type != AMDGPU_PMU_PERF_TYPE_ALL)
+		pr_info("Detected AMDGPU %s Counters. # of Counters = %d.\n",
+					pmu_type_name, total_num_events);
+	else
+		pr_info("Detected AMDGPU %d Perf Events.\n", total_num_events);
 
-	pr_info("Detected AMDGPU %s Counters. # of Counters = %d.\n",
-			pmu_type_name, num_counters);
 
 	list_add_tail(&pmu_entry->entry, &amdgpu_pmu_list);
 
 	return 0;
+err_register:
+	kfree(pmu_entry->pmu.attr_groups);
+err_attr_group:
+	kfree(pmu_entry->fmt_attr_group.attrs);
+	kfree(pmu_entry->fmt_attr);
+	kfree(pmu_entry->evt_attr_group.attrs);
+	kfree(pmu_entry->evt_attr);
+err_out:
+	pr_warn("Error initializing AMDGPU %s PMUs.\n", pmu_type_name);
+	return ret;
+}
+
+/* destroy all pmu data associated with target device */
+void amdgpu_pmu_fini(struct amdgpu_device *adev)
+{
+	struct amdgpu_pmu_entry *pe, *temp;
+
+	list_for_each_entry_safe(pe, temp, &amdgpu_pmu_list, entry) {
+		if (pe->adev != adev)
+			continue;
+		list_del(&pe->entry);
+		perf_pmu_unregister(&pe->pmu);
+		kfree(pe->pmu.attr_groups);
+		kfree(pe->fmt_attr_group.attrs);
+		kfree(pe->fmt_attr);
+		kfree(pe->evt_attr_group.attrs);
+		kfree(pe->evt_attr);
+		kfree(pe);
+	}
 }
 
 /* init amdgpu_pmu */
 int amdgpu_pmu_init(struct amdgpu_device *adev)
 {
 	int ret = 0;
+	struct amdgpu_pmu_entry *pmu_entry, *pmu_entry_df;
 
 	switch (adev->asic_type) {
 	case CHIP_VEGA20:
-		/* init df */
-		ret = init_pmu_by_type(adev, df_v3_6_attr_groups,
-				       "DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF,
-				       DF_V3_6_MAX_COUNTERS);
+		pmu_entry_df = kzalloc(sizeof(struct amdgpu_pmu_entry),
+								GFP_KERNEL);
+
+		if (!pmu_entry_df)
+			return -ENOMEM;
+
+		pmu_entry_df->adev = adev;
+		pmu_entry_df->fmt_attr_group.name = "format";
+		pmu_entry_df->fmt_attr_group.attrs = NULL;
+		pmu_entry_df->evt_attr_group.name = "events";
+		pmu_entry_df->evt_attr_group.attrs = NULL;
+		ret = init_pmu_by_type(pmu_entry_df, "DF", "amdgpu_df",
+						AMDGPU_PMU_PERF_TYPE_DF,
+						df_vega20_formats,
+						NUM_FORMATS_DF_VEGA20,
+						df_vega20_events,
+						NUM_EVENTS_DF_VEGA20,
+						NULL, 0);
+
+		if (ret) {
+			kfree(pmu_entry_df);
+			return ret;
+		}
 
-		/* other pmu types go here*/
-		break;
-	default:
-		return 0;
-	}
+		pmu_entry = kzalloc(sizeof(struct amdgpu_pmu_entry),
+								GFP_KERNEL);
 
-	return 0;
-}
+		if (!pmu_entry) {
+			amdgpu_pmu_fini(adev);
+			return -ENOMEM;
+		}
 
+		pmu_entry->adev = adev;
+		pmu_entry->fmt_attr_group.name = "format";
+		pmu_entry->fmt_attr_group.attrs = NULL;
+		pmu_entry->evt_attr_group.name = "events";
+		pmu_entry->evt_attr_group.attrs = NULL;
+		ret = init_pmu_by_type(pmu_entry, "", "amdgpu",
+						AMDGPU_PMU_PERF_TYPE_ALL,
+						amdgpu_pmu_formats,
+						NUM_FORMATS_AMDGPU_PMU,
+						vega20_events,
+						NUM_EVENTS_VEGA20_MAX,
+						vega20_event_config_types,
+						NUM_EVENT_TYPES_VEGA20);
+
+		if (ret) {
+			kfree(pmu_entry);
+			amdgpu_pmu_fini(adev);
+		}
 
-/* destroy all pmu data associated with target device */
-void amdgpu_pmu_fini(struct amdgpu_device *adev)
-{
-	struct amdgpu_pmu_entry *pe, *temp;
+		break;
+	default:
+		return 0;
+	};
 
-	list_for_each_entry_safe(pe, temp, &amdgpu_pmu_list, entry) {
-		if (pe->adev == adev) {
-			list_del(&pe->entry);
-			perf_pmu_unregister(&pe->pmu);
-			kfree(pe);
-		}
-	}
+	return ret;
 }
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.h
index 7dddb7160a11..08b9f7ca6375 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.h
@@ -26,11 +26,33 @@
 #ifndef _AMDGPU_PMU_H_
 #define _AMDGPU_PMU_H_
 
+/* PMU types. */
 enum amdgpu_pmu_perf_type {
-	PERF_TYPE_AMDGPU_DF = 0,
-	PERF_TYPE_AMDGPU_MAX
+	AMDGPU_PMU_PERF_TYPE_NONE = 0,
+	AMDGPU_PMU_PERF_TYPE_DF,
+	AMDGPU_PMU_PERF_TYPE_ALL
 };
 
+/*
+ * PMU type AMDGPU_PMU_PERF_TYPE_ALL can hold events of different "type"
+ * configurations.  Event config types are parsed from the 64-bit raw
+ * config (See EVENT_CONFIG_TYPE_SHIFT and EVENT_CONFIG_TYPE_MASK) and
+ * are registered into the HW perf events config_base.
+ *
+ * PMU types with only a single event configuration type
+ * (non-AMDGPU_PMU_PERF_TYPE_ALL) have their event config type auto generated
+ * when the performance counter is added.
+ */
+enum amdgpu_pmu_event_config_type {
+	AMDGPU_PMU_EVENT_CONFIG_TYPE_NONE = 0,
+	AMDGPU_PMU_EVENT_CONFIG_TYPE_DF,
+	AMDGPU_PMU_EVENT_CONFIG_TYPE_XGMI,
+	AMDGPU_PMU_EVENT_CONFIG_TYPE_MAX
+};
+
+#define AMDGPU_PMU_EVENT_CONFIG_TYPE_SHIFT	56
+#define AMDGPU_PMU_EVENT_CONFIG_TYPE_MASK	0xff
+
 int amdgpu_pmu_init(struct amdgpu_device *adev);
 void amdgpu_pmu_fini(struct amdgpu_device *adev);
 
diff --git a/drivers/gpu/drm/amd/amdgpu/df_v3_6.c b/drivers/gpu/drm/amd/amdgpu/df_v3_6.c
index 0ca6e176acb0..6e57ae95f997 100644
--- a/drivers/gpu/drm/amd/amdgpu/df_v3_6.c
+++ b/drivers/gpu/drm/amd/amdgpu/df_v3_6.c
@@ -30,71 +30,17 @@
 #define DF_3_6_SMN_REG_INST_DIST        0x8
 #define DF_3_6_INST_CNT                 8
 
-static u32 df_v3_6_channel_number[] = {1, 2, 0, 4, 0, 8, 0,
-				       16, 32, 0, 0, 0, 2, 4, 8};
-
-/* init df format attrs */
-AMDGPU_PMU_ATTR(event,		"config:0-7");
-AMDGPU_PMU_ATTR(instance,	"config:8-15");
-AMDGPU_PMU_ATTR(umask,		"config:16-23");
-
-/* df format attributes  */
-static struct attribute *df_v3_6_format_attrs[] = {
-	&pmu_attr_event.attr,
-	&pmu_attr_instance.attr,
-	&pmu_attr_umask.attr,
-	NULL
-};
-
-/* df format attribute group */
-static struct attribute_group df_v3_6_format_attr_group = {
-	.name = "format",
-	.attrs = df_v3_6_format_attrs,
-};
+/* Defined in global_features.h as FTI_PERFMON_VISIBLE */
+#define DF_V3_6_MAX_COUNTERS		4
 
-/* df event attrs */
-AMDGPU_PMU_ATTR(cake0_pcsout_txdata,
-		      "event=0x7,instance=0x46,umask=0x2");
-AMDGPU_PMU_ATTR(cake1_pcsout_txdata,
-		      "event=0x7,instance=0x47,umask=0x2");
-AMDGPU_PMU_ATTR(cake0_pcsout_txmeta,
-		      "event=0x7,instance=0x46,umask=0x4");
-AMDGPU_PMU_ATTR(cake1_pcsout_txmeta,
-		      "event=0x7,instance=0x47,umask=0x4");
-AMDGPU_PMU_ATTR(cake0_ftiinstat_reqalloc,
-		      "event=0xb,instance=0x46,umask=0x4");
-AMDGPU_PMU_ATTR(cake1_ftiinstat_reqalloc,
-		      "event=0xb,instance=0x47,umask=0x4");
-AMDGPU_PMU_ATTR(cake0_ftiinstat_rspalloc,
-		      "event=0xb,instance=0x46,umask=0x8");
-AMDGPU_PMU_ATTR(cake1_ftiinstat_rspalloc,
-		      "event=0xb,instance=0x47,umask=0x8");
-
-/* df event attributes  */
-static struct attribute *df_v3_6_event_attrs[] = {
-	&pmu_attr_cake0_pcsout_txdata.attr,
-	&pmu_attr_cake1_pcsout_txdata.attr,
-	&pmu_attr_cake0_pcsout_txmeta.attr,
-	&pmu_attr_cake1_pcsout_txmeta.attr,
-	&pmu_attr_cake0_ftiinstat_reqalloc.attr,
-	&pmu_attr_cake1_ftiinstat_reqalloc.attr,
-	&pmu_attr_cake0_ftiinstat_rspalloc.attr,
-	&pmu_attr_cake1_ftiinstat_rspalloc.attr,
-	NULL
-};
-
-/* df event attribute group */
-static struct attribute_group df_v3_6_event_attr_group = {
-	.name = "events",
-	.attrs = df_v3_6_event_attrs
-};
+/* get flags from df perfmon config */
+#define DF_V3_6_GET_EVENT(x)		(x & 0xFFUL)
+#define DF_V3_6_GET_INSTANCE(x)		((x >> 8) & 0xFFUL)
+#define DF_V3_6_GET_UNITMASK(x)		((x >> 16) & 0xFFUL)
+#define DF_V3_6_PERFMON_OVERFLOW	0xFFFFFFFFFFFFULL
 
-/* df event attr groups  */
-const struct attribute_group *df_v3_6_attr_groups[] = {
-		&df_v3_6_format_attr_group,
-		&df_v3_6_event_attr_group,
-		NULL
-};
+static u32 df_v3_6_channel_number[] = {1, 2, 0, 4, 0, 8, 0,
+				       16, 32, 0, 0, 0, 2, 4, 8};
 
 static uint64_t df_v3_6_get_fica(struct amdgpu_device *adev,
 				 uint32_t ficaa_val)
diff --git a/drivers/gpu/drm/amd/amdgpu/df_v3_6.h b/drivers/gpu/drm/amd/amdgpu/df_v3_6.h
index 76998541bc30..2505c7ef258a 100644
--- a/drivers/gpu/drm/amd/amdgpu/df_v3_6.h
+++ b/drivers/gpu/drm/amd/amdgpu/df_v3_6.h
@@ -35,15 +35,6 @@ enum DF_V3_6_MGCG {
 	DF_V3_6_MGCG_ENABLE_63_CYCLE_DELAY = 15
 };
 
-/* Defined in global_features.h as FTI_PERFMON_VISIBLE */
-#define DF_V3_6_MAX_COUNTERS		4
-
-/* get flags from df perfmon config */
-#define DF_V3_6_GET_EVENT(x)		(x & 0xFFUL)
-#define DF_V3_6_GET_INSTANCE(x)		((x >> 8) & 0xFFUL)
-#define DF_V3_6_GET_UNITMASK(x)		((x >> 16) & 0xFFUL)
-#define DF_V3_6_PERFMON_OVERFLOW	0xFFFFFFFFFFFFULL
-
 extern const struct attribute_group *df_v3_6_attr_groups[];
 extern const struct amdgpu_df_funcs df_v3_6_funcs;
 
-- 
2.17.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

RE: [PATCH 2/3] drm/amdgpu: add per device user friendly xgmi events for vega20

[Kim, Jonathan]

> -----Original Message-----
> From: Kasiviswanathan, Harish <Harish.Kasiviswanathan@amd.com>
> Sent: Monday, September 21, 2020 8:52 PM
> To: Kim, Jonathan <Jonathan.Kim@amd.com>; amd-gfx@lists.freedesktop.org
> Subject: RE: [PATCH 2/3] drm/amdgpu: add per device user friendly xgmi
> events for vega20
> 
> [AMD Official Use Only - Internal Distribution Only]
> 
> Few comments inline.
> 
> -----Original Message-----
> From: Kim, Jonathan <Jonathan.Kim@amd.com>
> Sent: Thursday, September 17, 2020 2:15 PM
> To: amd-gfx@lists.freedesktop.org
> Cc: Kasiviswanathan, Harish <Harish.Kasiviswanathan@amd.com>; Kim,
> Jonathan <Jonathan.Kim@amd.com>; Kim, Jonathan
> <Jonathan.Kim@amd.com>
> Subject: [PATCH 2/3] drm/amdgpu: add per device user friendly xgmi events
> for vega20
> 
> Non-outbound data metrics are non useful so mark them as legacy.
> Bucket new perf counters into device and not device ip.
> Bind events to chip instead of IP.
> Report available event counters and not number of hw counter banks.
> Move DF public macros to private since not needed outside of IP version.
> 
> v3: attr groups const array is global but attr groups are allocated per device
> which doesn't work and causes problems on memory allocation and de-
> allocation for pmu unregister. Switch to building const attr groups per pmu
> instead to simplify solution.
> 
> v2: add comments on sysfs structure and formatting.
> 
> Signed-off-by: Jonathan Kim <Jonathan.Kim@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu.h     |  13 -
>  drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c | 341
> ++++++++++++++++++++----
>  drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.h |   6 +-
>  drivers/gpu/drm/amd/amdgpu/df_v3_6.c    |  72 +----
>  drivers/gpu/drm/amd/amdgpu/df_v3_6.h    |   9 -
>  5 files changed, 304 insertions(+), 137 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index 13f92dea182a..f43dfdd2716a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -1279,19 +1279,6 @@ bool amdgpu_device_load_pci_state(struct pci_dev
> *pdev);
> 
>  #include "amdgpu_object.h"
> 
> -/* used by df_v3_6.c and amdgpu_pmu.c */
> -#define AMDGPU_PMU_ATTR(_name, _object)
> 	\
> -static ssize_t								\
> -_name##_show(struct device *dev,					\
> -			       struct device_attribute *attr,		\
> -			       char *page)				\
> -{									\
> -	BUILD_BUG_ON(sizeof(_object) >= PAGE_SIZE - 1);
> 	\
> -	return sprintf(page, _object "\n");				\
> -}									\
> -									\
> -static struct device_attribute pmu_attr_##_name = __ATTR_RO(_name)
> -
>  static inline bool amdgpu_is_tmz(struct amdgpu_device *adev)  {
>         return adev->gmc.tmz_enabled;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
> index 1b0ec715c8ba..74fe8fbdc0d1 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
> @@ -27,9 +27,19 @@
>  #include <linux/init.h>
>  #include "amdgpu.h"
>  #include "amdgpu_pmu.h"
> -#include "df_v3_6.h"
> 
>  #define PMU_NAME_SIZE 32
> +#define NUM_FORMATS_AMDGPU_PMU		4
> +#define NUM_FORMATS_DF_LEGACY		3
> +#define NUM_EVENTS_DF_LEGACY		8
> +#define NUM_EVENTS_VEGA20_XGMI		2
> +#define NUM_EVENTS_VEGA20_MAX		NUM_EVENTS_VEGA20_XGMI
> +
> +struct amdgpu_pmu_event_attribute {
> +	struct device_attribute attr;
> +	const char *event_str;
> +	unsigned int type;
> +};
> 
>  /* record to keep track of pmu entry per pmu type per device */  struct
> amdgpu_pmu_entry { @@ -37,10 +47,74 @@ struct amdgpu_pmu_entry {
>  	struct amdgpu_device *adev;
>  	struct pmu pmu;
>  	unsigned int pmu_perf_type;
> +	struct attribute_group fmt_attr_group;
> +	struct amdgpu_pmu_event_attribute *fmt_attr;
> +	struct attribute_group evt_attr_group;
> +	struct amdgpu_pmu_event_attribute *evt_attr;
>  };
> 
> +static ssize_t amdgpu_pmu_event_show(struct device *dev,
> +				struct device_attribute *attr, char *buf) {
> +	struct amdgpu_pmu_event_attribute *amdgpu_pmu_attr;
> +
> +	amdgpu_pmu_attr = container_of(attr, struct
> amdgpu_pmu_event_attribute,
> +									attr);
> +
> +	if (!amdgpu_pmu_attr->type)
> +		return sprintf(buf, "%s\n", amdgpu_pmu_attr->event_str);
> +
> +	return sprintf(buf, "%s,type=0x%x\n",
> +			amdgpu_pmu_attr->event_str, amdgpu_pmu_attr-
> >type); }
> +
>  static LIST_HEAD(amdgpu_pmu_list);
> 
> +/*
> + * Event formatting is global to all amdgpu events under sysfs folder
> + * /sys/bus/event_source/devices/amdgpu_<dev_num> where dev_num is the
> + * primary device index. Registered events can be found in subfolder "events"
> + * and formatting under subfolder "format".
> + *
> + * Formats "event", "instance", and "umask" are currently used by xGMI
> +but can
> + * be for generalized for other IP usage.  If format naming is
> +insufficient
> + * for newly registered IP events, append to the list below and handle
> +the
> + * perf events hardware configuration (see hwc->config) as required by the IP.
> + *
> + * Format "type" indicates IP type generated on pmu registration (see
> + * init_pmu_by_type) so non-legacy events omit this in the per-chip
> +event
> + * list (e.g. vega20_events).
> + */
> +static const char *amdgpu_pmu_formats[NUM_FORMATS_AMDGPU_PMU][2]
> = {
> +	{ "event", "config:0-7" },
> +	{ "instance", "config:8-15" },
> +	{ "umask", "config:16-23"},
> +	{ "type", "config:56-63"}
> +};
> +
> +/* Vega20 events */
> +static const char *vega20_events[NUM_EVENTS_VEGA20_MAX][2] = {
> +	{ "xgmi_link0_data_outbound", "event=0x7,instance=0x46,umask=0x2"
> },
> +	{ "xgmi_link1_data_outbound", "event=0x7,instance=0x47,umask=0x2"
> } };
> +
> +/* All df_vega20_* items are DEPRECATED. Use vega20_ items above
> +instead. */ static const char
> *df_vega20_formats[NUM_FORMATS_DF_LEGACY][2] = {
> +	{ "event", "config:0-7" },
> +	{ "instance", "config:8-15" },
> +	{ "umask", "config:16-23"}
> +};
> +
> +static const char *df_vega20_events[NUM_EVENTS_DF_LEGACY][2] = {
> +	{ "cake0_pcsout_txdata", "event=0x7,instance=0x46,umask=0x2" },
> +	{ "cake1_pcsout_txdata", "event=0x7,instance=0x47,umask=0x2" },
> +	{ "cake0_pcsout_txmeta", "event=0x7,instance=0x46,umask=0x4" },
> +	{ "cake1_pcsout_txmeta", "event=0x7,instance=0x47,umask=0x4" },
> +	{ "cake0_ftiinstat_reqalloc", "event=0xb,instance=0x46,umask=0x4" },
> +	{ "cake1_ftiinstat_reqalloc", "event=0xb,instance=0x47,umask=0x4" },
> +	{ "cake0_ftiinstat_rspalloc", "event=0xb,instance=0x46,umask=0x8" },
> +	{ "cake1_ftiinstat_rspalloc", "event=0xb,instance=0x47,umask=0x8" },
> +};
> 
>  /* initialize perf counter */
>  static int amdgpu_perf_event_init(struct perf_event *event) @@ -73,7 +147,8
> @@ static void amdgpu_perf_start(struct perf_event *event, int flags)
>  	hwc->state = 0;
> 
>  	switch (pe->pmu_perf_type) {
> -	case PERF_TYPE_AMDGPU_DF:
> +	case PERF_TYPE_AMDGPU_DF_LEGACY:
> +	case PERF_TYPE_AMDGPU_XGMI:
>  		if (!(flags & PERF_EF_RELOAD)) {
>  			target_cntr = pe->adev->df.funcs->pmc_start(pe-
> >adev,
>  						hwc->config, 0 /* unused */,
> @@ -108,7 +183,8 @@ static void amdgpu_perf_read(struct perf_event
> *event)
>  		prev = local64_read(&hwc->prev_count);
> 
>  		switch (pe->pmu_perf_type) {
> -		case PERF_TYPE_AMDGPU_DF:
> +		case PERF_TYPE_AMDGPU_DF_LEGACY:
> +		case PERF_TYPE_AMDGPU_XGMI:
>  			pe->adev->df.funcs->pmc_get_count(pe->adev,
>  						hwc->config, hwc->idx,
> &count);
>  			break;
> @@ -133,7 +209,8 @@ static void amdgpu_perf_stop(struct perf_event *event,
> int flags)
>  		return;
> 
>  	switch (pe->pmu_perf_type) {
> -	case PERF_TYPE_AMDGPU_DF:
> +	case PERF_TYPE_AMDGPU_DF_LEGACY:
> +	case PERF_TYPE_AMDGPU_XGMI:
>  		pe->adev->df.funcs->pmc_stop(pe->adev, hwc->config, hwc-
> >idx,
>  									0);
>  		break;
> @@ -160,10 +237,15 @@ static int amdgpu_perf_add(struct perf_event
> *event, int flags)
>  						  struct amdgpu_pmu_entry,
>  						  pmu);
> 
> +	if (pe->pmu_perf_type == PERF_TYPE_AMDGPU_MAX)
> +		pe->pmu_perf_type = (hwc->config >>
> AMDGPU_PERF_TYPE_SHIFT) &
> +
> 	AMDGPU_PERF_TYPE_MASK;
> +
>  	event->hw.state = PERF_HES_UPTODATE | PERF_HES_STOPPED;
> 
>  	switch (pe->pmu_perf_type) {
> -	case PERF_TYPE_AMDGPU_DF:
> +	case PERF_TYPE_AMDGPU_DF_LEGACY:
> +	case PERF_TYPE_AMDGPU_XGMI:
>  		target_cntr = pe->adev->df.funcs->pmc_start(pe->adev,
>  						hwc->config, 0 /* unused */,
>  						1 /* add counter */);
> @@ -197,7 +279,8 @@ static void amdgpu_perf_del(struct perf_event *event,
> int flags)
>  	amdgpu_perf_stop(event, PERF_EF_UPDATE);
> 
>  	switch (pe->pmu_perf_type) {
> -	case PERF_TYPE_AMDGPU_DF:
> +	case PERF_TYPE_AMDGPU_DF_LEGACY:
> +	case PERF_TYPE_AMDGPU_XGMI:
>  		pe->adev->df.funcs->pmc_stop(pe->adev, hwc->config, hwc-
> >idx,
>  									1);
>  		break;
> @@ -208,25 +291,83 @@ static void amdgpu_perf_del(struct perf_event
> *event, int flags)
>  	perf_event_update_userpage(event);
>  }
> 
> -/* vega20 pmus */
> -
> -/* init pmu tracking per pmu type */
> -static int init_pmu_by_type(struct amdgpu_device *adev,
> -		  const struct attribute_group *attr_groups[],
> -		  char *pmu_type_name, char *pmu_file_prefix,
> -		  unsigned int pmu_perf_type,
> -		  unsigned int num_counters)
> +static void amdgpu_pmu_create_attributes(struct attribute_group
> *attr_group,
> +				struct amdgpu_pmu_event_attribute
> *pmu_attr,
> +				const char *events[][2],
> +				int s_offset,
> +				int e_offset,
> +				unsigned int type)
> 
> Is s_offset really required. I see 0 is passed in all cases.

Right now it's 0 because we only have xGMI events. To add other event types to the events array, s_offset is needed because we have to call this function per event type in /* other events can be added here */ (I should probably update this comment to reflect that).  So pmu_attr needs to know where it left off from the last event type creation per pmu registration.  For formats, offsets aren't necessary because they cover all event types so it's called only once per ASIC.  Maybe to avoid misuse on that point, I'll split amdgpu_pmu_create_attribute into amdgpu_pmu_create_event_attrs_by_type and have a new amdgpu_pmu_create_format_attrs be a wrapper on amdgpu_pmu_create_event_attrs_by_type with s_offset and type set to 0.  

> 
> 
> -	char pmu_name[PMU_NAME_SIZE];
> -	struct amdgpu_pmu_entry *pmu_entry;
> -	int ret = 0;
> +	int i;
> +
> +	pmu_attr += s_offset;
> +
> +	for (i = s_offset; i < e_offset; i++) {
> +		attr_group->attrs[i] = &pmu_attr->attr.attr;
> +		sysfs_attr_init(&pmu_attr->attr.attr);
> +		pmu_attr->attr.attr.name = events[i][0];
> +		pmu_attr->attr.attr.mode = 0444;
> +		pmu_attr->attr.show = amdgpu_pmu_event_show;
> +		pmu_attr->event_str = events[i][1];
> +		pmu_attr->type = type;
> +		pmu_attr++;
> +	}
> +}
> 
> -	pmu_entry = kzalloc(sizeof(struct amdgpu_pmu_entry), GFP_KERNEL);
> +static int amdgpu_pmu_alloc_pmu_attrs(
> +				struct attribute_group *fmt_attr_group,
> +				struct amdgpu_pmu_event_attribute
> **fmt_attr,
> +				int fmt_num_attrs,
> +				struct attribute_group *evt_attr_group,
> +				struct amdgpu_pmu_event_attribute
> **evt_attr,
> +				int evt_num_attrs)
> +{
> +	*fmt_attr = kcalloc(fmt_num_attrs, sizeof(**fmt_attr), GFP_KERNEL);
> 
> -	if (!pmu_entry)
> +	if (!(*fmt_attr))
>  		return -ENOMEM;
> 
> -	pmu_entry->adev = adev;
> +	fmt_attr_group->attrs = kcalloc(fmt_num_attrs + 1,
> +				sizeof(*fmt_attr_group->attrs), GFP_KERNEL);
> +
> +	if (!fmt_attr_group->attrs)
> +		goto err_fmt_attr_grp;
> +
> +	*evt_attr = kcalloc(evt_num_attrs, sizeof(**evt_attr), GFP_KERNEL);
> +
> +	if (!(*evt_attr))
> +		goto err_evt_attr;
> +
> +	evt_attr_group->attrs = kcalloc(evt_num_attrs + 1,
> +				sizeof(*evt_attr_group->attrs), GFP_KERNEL);
> +
> +	if (!evt_attr_group->attrs)
> +		goto err_evt_attr_grp;
> +
> +	return 0;
> +err_evt_attr_grp:
> +	kfree(*evt_attr);
> +err_evt_attr:
> +	kfree(fmt_attr_group->attrs);
> +err_fmt_attr_grp:
> +	kfree(*fmt_attr);
> +	return -ENOMEM;
> +}
> +
> +/* init pmu tracking per pmu type */
> +static int init_pmu_by_type(struct amdgpu_pmu_entry *pmu_entry,
> +			char *pmu_type_name, char *pmu_file_prefix,
> +			unsigned int pmu_perf_type)
> 
> I feel this function could be a simpler ASIC independent helper function. If you
> check all asic_types instead of just VEGA20 in amdgpu_pmu_init(), this function
> could be something like this init_pmu_by_type(struct amdgpu_pmu_entry
> *pmu_entry,
> 			char *pmu_type_name, char *pmu_file_prefix,
> 			unsigned int pmu_perf_type,
> 			const char *events[][2], int num_of_events,
> 			const char *fmts[][2], int mum_of_events); This way
> you could probably avoid all "is_legacy ?" cases

My first comment should answer why it's necessary to arbitrate by asic_type in init_pmu_by_type.  We don't know how to create attributes per event type per registration unless we know the ASIC.  Your suggestion would limit us to a single event type per registration, similar to how vega20 DF legacy is constructed - amdgpu_df_<num>, which is what we're trying to get away from.  We need to dynamically set the "type" field per event because we can't re-register the pmu attribute groups later with new events (throws EEXIST).  This is why Vega20 registers twice (single event type per pmu as legacy and the new xgmi events as per event type per pmu) while other ASICS will only register once. 
I think the problem is the pmu_perf_type arg is getting mixed up with per-event type with the abuse of the enum.  I'll change this to make that clear.

> 
> 
> +{
> +	const struct attribute_group *attr_groups[] = {
> +		&pmu_entry->fmt_attr_group,
> +		&pmu_entry->evt_attr_group,
> +		NULL
> +	};
> +	char pmu_name[PMU_NAME_SIZE];
> +	bool is_legacy = pmu_perf_type == PERF_TYPE_AMDGPU_DF_LEGACY;
> +	int ret = 0, num_events = 0;
> +
>  	pmu_entry->pmu = (struct pmu){
>  		.event_init = amdgpu_perf_event_init,
>  		.add = amdgpu_perf_add,
> @@ -237,59 +378,157 @@ static int init_pmu_by_type(struct amdgpu_device
> *adev,
>  		.task_ctx_nr = perf_invalid_context,
>  	};
> 
> -	pmu_entry->pmu.attr_groups = attr_groups;
> +	switch (pmu_entry->adev->asic_type) {
> +	case CHIP_VEGA20:
> +		ret = amdgpu_pmu_alloc_pmu_attrs(&pmu_entry-
> >fmt_attr_group,
> +					&pmu_entry->fmt_attr,
> +					is_legacy ?
> NUM_FORMATS_DF_LEGACY :
> +
> 	NUM_FORMATS_AMDGPU_PMU,
> +					&pmu_entry->evt_attr_group,
> +					&pmu_entry->evt_attr,
> +					is_legacy ? NUM_EVENTS_DF_LEGACY
> :
> +
> 	NUM_EVENTS_VEGA20_MAX);
> +
> +		if (ret)
> +			goto err_out;
> +
> +		amdgpu_pmu_create_attributes(&pmu_entry-
> >fmt_attr_group,
> +					pmu_entry->fmt_attr,
> +					is_legacy ? df_vega20_formats :
> +
> 	amdgpu_pmu_formats, 0,
> +					is_legacy ?
> NUM_FORMATS_DF_LEGACY :
> +
> 	NUM_FORMATS_AMDGPU_PMU,
> +					0);
> +
> +		amdgpu_pmu_create_attributes(&pmu_entry->evt_attr_group,
> +					pmu_entry->evt_attr,
> +					is_legacy ? df_vega20_events :
> +							vega20_events, 0,
> +					is_legacy ? NUM_EVENTS_DF_LEGACY
> :
> +
> 	NUM_EVENTS_VEGA20_XGMI,
> +					is_legacy ?
> PERF_TYPE_AMDGPU_DF_LEGACY :
> +
> 	PERF_TYPE_AMDGPU_XGMI);
> +		num_events += is_legacy ? NUM_EVENTS_DF_LEGACY :
> +
> 	NUM_EVENTS_VEGA20_XGMI;
> +
> 
> Are you using += here in preparation for future events. Because this variable
> num_events is used only once in this function

Yes.  I'll rename this to total_num_events to make that clear.

> 
> +		/* other events can be added here */
> +
> +		break;
> +	default:
> +		ret = -ENODEV;
> +		goto err_out;
> +	};
> +
> +	pmu_entry->pmu.attr_groups = kmemdup(attr_groups,
> sizeof(attr_groups),
> +								GFP_KERNEL);
> +
> +	if (!pmu_entry->pmu.attr_groups)
> +		goto err_attr_group;
> +
>  	pmu_entry->pmu_perf_type = pmu_perf_type;
> -	snprintf(pmu_name, PMU_NAME_SIZE, "%s_%d",
> -				pmu_file_prefix, adev_to_drm(adev)-
> >primary->index);
> +	snprintf(pmu_name, PMU_NAME_SIZE, "%s_%d", pmu_file_prefix,
> +				adev_to_drm(pmu_entry->adev)->primary-
> >index);
> 
>  	ret = perf_pmu_register(&pmu_entry->pmu, pmu_name, -1);
> 
> -	if (ret) {
> -		kfree(pmu_entry);
> -		pr_warn("Error initializing AMDGPU %s PMUs.\n",
> pmu_type_name);
> -		return ret;
> -	}
> +	if (ret)
> +		goto err_register;
> 
>  	pr_info("Detected AMDGPU %s Counters. # of Counters = %d.\n",
> -			pmu_type_name, num_counters);
> +			pmu_type_name, num_events);
> 
> Should the "# of Counters" be changes to "# of Events"

Then maybe we should do "# of Perf Events" instead to be explicit that we're using perf_events for non-legacy cases.  I think there are a bunch of other events tracked that don't use perf_events in our driver.

Thanks,

Jon

> 
>  	list_add_tail(&pmu_entry->entry, &amdgpu_pmu_list);
> 
>  	return 0;
> +err_register:
> +	kfree(pmu_entry->pmu.attr_groups);
> +err_attr_group:
> +	kfree(pmu_entry->fmt_attr_group.attrs);
> +	kfree(pmu_entry->fmt_attr);
> +	kfree(pmu_entry->evt_attr_group.attrs);
> +	kfree(pmu_entry->evt_attr);
> +err_out:
> +	pr_warn("Error initializing AMDGPU %s PMUs.\n", pmu_type_name);
> +	return ret;
> +}
> +
> +/* destroy all pmu data associated with target device */ void
> +amdgpu_pmu_fini(struct amdgpu_device *adev) {
> +	struct amdgpu_pmu_entry *pe, *temp;
> +
> +	list_for_each_entry_safe(pe, temp, &amdgpu_pmu_list, entry) {
> +		if (pe->adev != adev)
> +			continue;
> +		list_del(&pe->entry);
> +		perf_pmu_unregister(&pe->pmu);
> +		kfree(pe->pmu.attr_groups);
> +		kfree(pe->fmt_attr_group.attrs);
> +		kfree(pe->fmt_attr);
> +		kfree(pe->evt_attr_group.attrs);
> +		kfree(pe->evt_attr);
> +		kfree(pe);
> +	}
> +}
> +
> +static bool amdgpu_pmu_is_supported(struct amdgpu_device *adev) {
> +	return adev->asic_type == CHIP_VEGA20;
>  }
> 
>  /* init amdgpu_pmu */
>  int amdgpu_pmu_init(struct amdgpu_device *adev)  {
>  	int ret = 0;
> +	struct amdgpu_pmu_entry *pmu_entry, *pmu_entry_legacy;
> 
> -	switch (adev->asic_type) {
> -	case CHIP_VEGA20:
> -		/* init df */
> -		ret = init_pmu_by_type(adev, df_v3_6_attr_groups,
> -				       "DF", "amdgpu_df",
> PERF_TYPE_AMDGPU_DF,
> -				       DF_V3_6_MAX_COUNTERS);
> -
> -		/* other pmu types go here*/
> -		break;
> -	default:
> +	if (!amdgpu_pmu_is_supported(adev))
>  		return 0;
> -	}
> 
> -	return 0;
> -}
> +	if (adev->asic_type == CHIP_VEGA20) {
> +		pmu_entry_legacy = kzalloc(sizeof(struct amdgpu_pmu_entry),
> +								GFP_KERNEL);
> 
> +		if (!pmu_entry_legacy)
> +			return -ENOMEM;
> 
> -/* destroy all pmu data associated with target device */ -void
> amdgpu_pmu_fini(struct amdgpu_device *adev) -{
> -	struct amdgpu_pmu_entry *pe, *temp;
> +		pmu_entry_legacy->adev = adev;
> +		pmu_entry_legacy->fmt_attr_group.name = "format";
> +		pmu_entry_legacy->fmt_attr_group.attrs = NULL;
> +		pmu_entry_legacy->evt_attr_group.name = "events";
> +		pmu_entry_legacy->evt_attr_group.attrs = NULL;
> 
> -	list_for_each_entry_safe(pe, temp, &amdgpu_pmu_list, entry) {
> -		if (pe->adev == adev) {
> -			list_del(&pe->entry);
> -			perf_pmu_unregister(&pe->pmu);
> -			kfree(pe);
> +		ret = init_pmu_by_type(pmu_entry_legacy, "DF", "amdgpu_df",
> +
> 	PERF_TYPE_AMDGPU_DF_LEGACY);
> +
> +		if (ret) {
> +			kfree(pmu_entry_legacy);
> +			return ret;
>  		}
>  	}
> +
> +	pmu_entry = kzalloc(sizeof(struct amdgpu_pmu_entry), GFP_KERNEL);
> +
> +	if (!pmu_entry) {
> +		if (adev->asic_type == CHIP_VEGA20)
> +			amdgpu_pmu_fini(adev);
> +		return -ENOMEM;
> +	}
> +
> +	pmu_entry->adev = adev;
> +	pmu_entry->fmt_attr_group.name = "format";
> +	pmu_entry->fmt_attr_group.attrs = NULL;
> +	pmu_entry->evt_attr_group.name = "events";
> +	pmu_entry->evt_attr_group.attrs = NULL;
> +
> +	ret = init_pmu_by_type(pmu_entry, "Event", "amdgpu",
> +
> 	PERF_TYPE_AMDGPU_MAX);
> +	if (ret) {
> +		if (adev->asic_type == CHIP_VEGA20)
> +			amdgpu_pmu_fini(adev);
> +		kfree(pmu_entry);
> +
> +	}
> +
> +	return ret;
>  }
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.h
> index 7dddb7160a11..0d214abe720e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.h
> @@ -27,10 +27,14 @@
>  #define _AMDGPU_PMU_H_
> 
>  enum amdgpu_pmu_perf_type {
> -	PERF_TYPE_AMDGPU_DF = 0,
> +	PERF_TYPE_AMDGPU_DF_LEGACY = 0,
> +	PERF_TYPE_AMDGPU_XGMI,
>  	PERF_TYPE_AMDGPU_MAX
>  };
> 
> +#define AMDGPU_PERF_TYPE_SHIFT	56
> +#define AMDGPU_PERF_TYPE_MASK	0xff
> +
>  int amdgpu_pmu_init(struct amdgpu_device *adev);  void
> amdgpu_pmu_fini(struct amdgpu_device *adev);
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/df_v3_6.c
> b/drivers/gpu/drm/amd/amdgpu/df_v3_6.c
> index 0ca6e176acb0..6e57ae95f997 100644
> --- a/drivers/gpu/drm/amd/amdgpu/df_v3_6.c
> +++ b/drivers/gpu/drm/amd/amdgpu/df_v3_6.c
> @@ -30,71 +30,17 @@
>  #define DF_3_6_SMN_REG_INST_DIST        0x8
>  #define DF_3_6_INST_CNT                 8
> 
> -static u32 df_v3_6_channel_number[] = {1, 2, 0, 4, 0, 8, 0,
> -				       16, 32, 0, 0, 0, 2, 4, 8};
> -
> -/* init df format attrs */
> -AMDGPU_PMU_ATTR(event,		"config:0-7");
> -AMDGPU_PMU_ATTR(instance,	"config:8-15");
> -AMDGPU_PMU_ATTR(umask,		"config:16-23");
> -
> -/* df format attributes  */
> -static struct attribute *df_v3_6_format_attrs[] = {
> -	&pmu_attr_event.attr,
> -	&pmu_attr_instance.attr,
> -	&pmu_attr_umask.attr,
> -	NULL
> -};
> -
> -/* df format attribute group */
> -static struct attribute_group df_v3_6_format_attr_group = {
> -	.name = "format",
> -	.attrs = df_v3_6_format_attrs,
> -};
> +/* Defined in global_features.h as FTI_PERFMON_VISIBLE */
> +#define DF_V3_6_MAX_COUNTERS		4
> 
> -/* df event attrs */
> -AMDGPU_PMU_ATTR(cake0_pcsout_txdata,
> -		      "event=0x7,instance=0x46,umask=0x2");
> -AMDGPU_PMU_ATTR(cake1_pcsout_txdata,
> -		      "event=0x7,instance=0x47,umask=0x2");
> -AMDGPU_PMU_ATTR(cake0_pcsout_txmeta,
> -		      "event=0x7,instance=0x46,umask=0x4");
> -AMDGPU_PMU_ATTR(cake1_pcsout_txmeta,
> -		      "event=0x7,instance=0x47,umask=0x4");
> -AMDGPU_PMU_ATTR(cake0_ftiinstat_reqalloc,
> -		      "event=0xb,instance=0x46,umask=0x4");
> -AMDGPU_PMU_ATTR(cake1_ftiinstat_reqalloc,
> -		      "event=0xb,instance=0x47,umask=0x4");
> -AMDGPU_PMU_ATTR(cake0_ftiinstat_rspalloc,
> -		      "event=0xb,instance=0x46,umask=0x8");
> -AMDGPU_PMU_ATTR(cake1_ftiinstat_rspalloc,
> -		      "event=0xb,instance=0x47,umask=0x8");
> -
> -/* df event attributes  */
> -static struct attribute *df_v3_6_event_attrs[] = {
> -	&pmu_attr_cake0_pcsout_txdata.attr,
> -	&pmu_attr_cake1_pcsout_txdata.attr,
> -	&pmu_attr_cake0_pcsout_txmeta.attr,
> -	&pmu_attr_cake1_pcsout_txmeta.attr,
> -	&pmu_attr_cake0_ftiinstat_reqalloc.attr,
> -	&pmu_attr_cake1_ftiinstat_reqalloc.attr,
> -	&pmu_attr_cake0_ftiinstat_rspalloc.attr,
> -	&pmu_attr_cake1_ftiinstat_rspalloc.attr,
> -	NULL
> -};
> -
> -/* df event attribute group */
> -static struct attribute_group df_v3_6_event_attr_group = {
> -	.name = "events",
> -	.attrs = df_v3_6_event_attrs
> -};
> +/* get flags from df perfmon config */
> +#define DF_V3_6_GET_EVENT(x)		(x & 0xFFUL)
> +#define DF_V3_6_GET_INSTANCE(x)		((x >> 8) & 0xFFUL)
> +#define DF_V3_6_GET_UNITMASK(x)		((x >> 16) & 0xFFUL)
> +#define DF_V3_6_PERFMON_OVERFLOW	0xFFFFFFFFFFFFULL
> 
> -/* df event attr groups  */
> -const struct attribute_group *df_v3_6_attr_groups[] = {
> -		&df_v3_6_format_attr_group,
> -		&df_v3_6_event_attr_group,
> -		NULL
> -};
> +static u32 df_v3_6_channel_number[] = {1, 2, 0, 4, 0, 8, 0,
> +				       16, 32, 0, 0, 0, 2, 4, 8};
> 
>  static uint64_t df_v3_6_get_fica(struct amdgpu_device *adev,
>  				 uint32_t ficaa_val)
> diff --git a/drivers/gpu/drm/amd/amdgpu/df_v3_6.h
> b/drivers/gpu/drm/amd/amdgpu/df_v3_6.h
> index 76998541bc30..2505c7ef258a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/df_v3_6.h
> +++ b/drivers/gpu/drm/amd/amdgpu/df_v3_6.h
> @@ -35,15 +35,6 @@ enum DF_V3_6_MGCG {
>  	DF_V3_6_MGCG_ENABLE_63_CYCLE_DELAY = 15  };
> 
> -/* Defined in global_features.h as FTI_PERFMON_VISIBLE */
> -#define DF_V3_6_MAX_COUNTERS		4
> -
> -/* get flags from df perfmon config */
> -#define DF_V3_6_GET_EVENT(x)		(x & 0xFFUL)
> -#define DF_V3_6_GET_INSTANCE(x)		((x >> 8) & 0xFFUL)
> -#define DF_V3_6_GET_UNITMASK(x)		((x >> 16) & 0xFFUL)
> -#define DF_V3_6_PERFMON_OVERFLOW	0xFFFFFFFFFFFFULL
> -
>  extern const struct attribute_group *df_v3_6_attr_groups[];  extern const
> struct amdgpu_df_funcs df_v3_6_funcs;
> 
> --
> 2.17.1
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

RE: [PATCH 2/3] drm/amdgpu: add per device user friendly xgmi events for vega20

[Kasiviswanathan, Harish]

[AMD Official Use Only - Internal Distribution Only]

Few comments inline.

-----Original Message-----
From: Kim, Jonathan <Jonathan.Kim@amd.com> 
Sent: Thursday, September 17, 2020 2:15 PM
To: amd-gfx@lists.freedesktop.org
Cc: Kasiviswanathan, Harish <Harish.Kasiviswanathan@amd.com>; Kim, Jonathan <Jonathan.Kim@amd.com>; Kim, Jonathan <Jonathan.Kim@amd.com>
Subject: [PATCH 2/3] drm/amdgpu: add per device user friendly xgmi events for vega20

Non-outbound data metrics are non useful so mark them as legacy.
Bucket new perf counters into device and not device ip.
Bind events to chip instead of IP.
Report available event counters and not number of hw counter banks.
Move DF public macros to private since not needed outside of IP version.

v3: attr groups const array is global but attr groups are allocated per
device which doesn't work and causes problems on memory allocation and
de-allocation for pmu unregister. Switch to building const attr groups
per pmu instead to simplify solution.

v2: add comments on sysfs structure and formatting.

Signed-off-by: Jonathan Kim <Jonathan.Kim@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h     |  13 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c | 341 ++++++++++++++++++++----
 drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.h |   6 +-
 drivers/gpu/drm/amd/amdgpu/df_v3_6.c    |  72 +----
 drivers/gpu/drm/amd/amdgpu/df_v3_6.h    |   9 -
 5 files changed, 304 insertions(+), 137 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 13f92dea182a..f43dfdd2716a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1279,19 +1279,6 @@ bool amdgpu_device_load_pci_state(struct pci_dev *pdev);
 
 #include "amdgpu_object.h"
 
-/* used by df_v3_6.c and amdgpu_pmu.c */
-#define AMDGPU_PMU_ATTR(_name, _object)					\
-static ssize_t								\
-_name##_show(struct device *dev,					\
-			       struct device_attribute *attr,		\
-			       char *page)				\
-{									\
-	BUILD_BUG_ON(sizeof(_object) >= PAGE_SIZE - 1);			\
-	return sprintf(page, _object "\n");				\
-}									\
-									\
-static struct device_attribute pmu_attr_##_name = __ATTR_RO(_name)
-
 static inline bool amdgpu_is_tmz(struct amdgpu_device *adev)
 {
        return adev->gmc.tmz_enabled;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
index 1b0ec715c8ba..74fe8fbdc0d1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
@@ -27,9 +27,19 @@
 #include <linux/init.h>
 #include "amdgpu.h"
 #include "amdgpu_pmu.h"
-#include "df_v3_6.h"
 
 #define PMU_NAME_SIZE 32
+#define NUM_FORMATS_AMDGPU_PMU		4
+#define NUM_FORMATS_DF_LEGACY		3
+#define NUM_EVENTS_DF_LEGACY		8
+#define NUM_EVENTS_VEGA20_XGMI		2
+#define NUM_EVENTS_VEGA20_MAX		NUM_EVENTS_VEGA20_XGMI
+
+struct amdgpu_pmu_event_attribute {
+	struct device_attribute attr;
+	const char *event_str;
+	unsigned int type;
+};
 
 /* record to keep track of pmu entry per pmu type per device */
 struct amdgpu_pmu_entry {
@@ -37,10 +47,74 @@ struct amdgpu_pmu_entry {
 	struct amdgpu_device *adev;
 	struct pmu pmu;
 	unsigned int pmu_perf_type;
+	struct attribute_group fmt_attr_group;
+	struct amdgpu_pmu_event_attribute *fmt_attr;
+	struct attribute_group evt_attr_group;
+	struct amdgpu_pmu_event_attribute *evt_attr;
 };
 
+static ssize_t amdgpu_pmu_event_show(struct device *dev,
+				struct device_attribute *attr, char *buf)
+{
+	struct amdgpu_pmu_event_attribute *amdgpu_pmu_attr;
+
+	amdgpu_pmu_attr = container_of(attr, struct amdgpu_pmu_event_attribute,
+									attr);
+
+	if (!amdgpu_pmu_attr->type)
+		return sprintf(buf, "%s\n", amdgpu_pmu_attr->event_str);
+
+	return sprintf(buf, "%s,type=0x%x\n",
+			amdgpu_pmu_attr->event_str, amdgpu_pmu_attr->type);
+}
+
 static LIST_HEAD(amdgpu_pmu_list);
 
+/*
+ * Event formatting is global to all amdgpu events under sysfs folder
+ * /sys/bus/event_source/devices/amdgpu_<dev_num> where dev_num is the
+ * primary device index. Registered events can be found in subfolder "events"
+ * and formatting under subfolder "format".
+ *
+ * Formats "event", "instance", and "umask" are currently used by xGMI but can
+ * be for generalized for other IP usage.  If format naming is insufficient
+ * for newly registered IP events, append to the list below and handle the
+ * perf events hardware configuration (see hwc->config) as required by the IP.
+ *
+ * Format "type" indicates IP type generated on pmu registration (see
+ * init_pmu_by_type) so non-legacy events omit this in the per-chip event
+ * list (e.g. vega20_events).
+ */
+static const char *amdgpu_pmu_formats[NUM_FORMATS_AMDGPU_PMU][2] = {
+	{ "event", "config:0-7" },
+	{ "instance", "config:8-15" },
+	{ "umask", "config:16-23"},
+	{ "type", "config:56-63"}
+};
+
+/* Vega20 events */
+static const char *vega20_events[NUM_EVENTS_VEGA20_MAX][2] = {
+	{ "xgmi_link0_data_outbound", "event=0x7,instance=0x46,umask=0x2" },
+	{ "xgmi_link1_data_outbound", "event=0x7,instance=0x47,umask=0x2" }
+};
+
+/* All df_vega20_* items are DEPRECATED. Use vega20_ items above instead. */
+static const char *df_vega20_formats[NUM_FORMATS_DF_LEGACY][2] = {
+	{ "event", "config:0-7" },
+	{ "instance", "config:8-15" },
+	{ "umask", "config:16-23"}
+};
+
+static const char *df_vega20_events[NUM_EVENTS_DF_LEGACY][2] = {
+	{ "cake0_pcsout_txdata", "event=0x7,instance=0x46,umask=0x2" },
+	{ "cake1_pcsout_txdata", "event=0x7,instance=0x47,umask=0x2" },
+	{ "cake0_pcsout_txmeta", "event=0x7,instance=0x46,umask=0x4" },
+	{ "cake1_pcsout_txmeta", "event=0x7,instance=0x47,umask=0x4" },
+	{ "cake0_ftiinstat_reqalloc", "event=0xb,instance=0x46,umask=0x4" },
+	{ "cake1_ftiinstat_reqalloc", "event=0xb,instance=0x47,umask=0x4" },
+	{ "cake0_ftiinstat_rspalloc", "event=0xb,instance=0x46,umask=0x8" },
+	{ "cake1_ftiinstat_rspalloc", "event=0xb,instance=0x47,umask=0x8" },
+};
 
 /* initialize perf counter */
 static int amdgpu_perf_event_init(struct perf_event *event)
@@ -73,7 +147,8 @@ static void amdgpu_perf_start(struct perf_event *event, int flags)
 	hwc->state = 0;
 
 	switch (pe->pmu_perf_type) {
-	case PERF_TYPE_AMDGPU_DF:
+	case PERF_TYPE_AMDGPU_DF_LEGACY:
+	case PERF_TYPE_AMDGPU_XGMI:
 		if (!(flags & PERF_EF_RELOAD)) {
 			target_cntr = pe->adev->df.funcs->pmc_start(pe->adev,
 						hwc->config, 0 /* unused */,
@@ -108,7 +183,8 @@ static void amdgpu_perf_read(struct perf_event *event)
 		prev = local64_read(&hwc->prev_count);
 
 		switch (pe->pmu_perf_type) {
-		case PERF_TYPE_AMDGPU_DF:
+		case PERF_TYPE_AMDGPU_DF_LEGACY:
+		case PERF_TYPE_AMDGPU_XGMI:
 			pe->adev->df.funcs->pmc_get_count(pe->adev,
 						hwc->config, hwc->idx, &count);
 			break;
@@ -133,7 +209,8 @@ static void amdgpu_perf_stop(struct perf_event *event, int flags)
 		return;
 
 	switch (pe->pmu_perf_type) {
-	case PERF_TYPE_AMDGPU_DF:
+	case PERF_TYPE_AMDGPU_DF_LEGACY:
+	case PERF_TYPE_AMDGPU_XGMI:
 		pe->adev->df.funcs->pmc_stop(pe->adev, hwc->config, hwc->idx,
 									0);
 		break;
@@ -160,10 +237,15 @@ static int amdgpu_perf_add(struct perf_event *event, int flags)
 						  struct amdgpu_pmu_entry,
 						  pmu);
 
+	if (pe->pmu_perf_type == PERF_TYPE_AMDGPU_MAX)
+		pe->pmu_perf_type = (hwc->config >> AMDGPU_PERF_TYPE_SHIFT) &
+							AMDGPU_PERF_TYPE_MASK;
+
 	event->hw.state = PERF_HES_UPTODATE | PERF_HES_STOPPED;
 
 	switch (pe->pmu_perf_type) {
-	case PERF_TYPE_AMDGPU_DF:
+	case PERF_TYPE_AMDGPU_DF_LEGACY:
+	case PERF_TYPE_AMDGPU_XGMI:
 		target_cntr = pe->adev->df.funcs->pmc_start(pe->adev,
 						hwc->config, 0 /* unused */,
 						1 /* add counter */);
@@ -197,7 +279,8 @@ static void amdgpu_perf_del(struct perf_event *event, int flags)
 	amdgpu_perf_stop(event, PERF_EF_UPDATE);
 
 	switch (pe->pmu_perf_type) {
-	case PERF_TYPE_AMDGPU_DF:
+	case PERF_TYPE_AMDGPU_DF_LEGACY:
+	case PERF_TYPE_AMDGPU_XGMI:
 		pe->adev->df.funcs->pmc_stop(pe->adev, hwc->config, hwc->idx,
 									1);
 		break;
@@ -208,25 +291,83 @@ static void amdgpu_perf_del(struct perf_event *event, int flags)
 	perf_event_update_userpage(event);
 }
 
-/* vega20 pmus */
-
-/* init pmu tracking per pmu type */
-static int init_pmu_by_type(struct amdgpu_device *adev,
-		  const struct attribute_group *attr_groups[],
-		  char *pmu_type_name, char *pmu_file_prefix,
-		  unsigned int pmu_perf_type,
-		  unsigned int num_counters)
+static void amdgpu_pmu_create_attributes(struct attribute_group *attr_group,
+				struct amdgpu_pmu_event_attribute *pmu_attr,
+				const char *events[][2],
+				int s_offset,
+				int e_offset,
+				unsigned int type)

Is s_offset really required. I see 0 is passed in all cases.

 {
-	char pmu_name[PMU_NAME_SIZE];
-	struct amdgpu_pmu_entry *pmu_entry;
-	int ret = 0;
+	int i;
+
+	pmu_attr += s_offset;
+
+	for (i = s_offset; i < e_offset; i++) {
+		attr_group->attrs[i] = &pmu_attr->attr.attr;
+		sysfs_attr_init(&pmu_attr->attr.attr);
+		pmu_attr->attr.attr.name = events[i][0];
+		pmu_attr->attr.attr.mode = 0444;
+		pmu_attr->attr.show = amdgpu_pmu_event_show;
+		pmu_attr->event_str = events[i][1];
+		pmu_attr->type = type;
+		pmu_attr++;
+	}
+}
 
-	pmu_entry = kzalloc(sizeof(struct amdgpu_pmu_entry), GFP_KERNEL);
+static int amdgpu_pmu_alloc_pmu_attrs(
+				struct attribute_group *fmt_attr_group,
+				struct amdgpu_pmu_event_attribute **fmt_attr,
+				int fmt_num_attrs,
+				struct attribute_group *evt_attr_group,
+				struct amdgpu_pmu_event_attribute **evt_attr,
+				int evt_num_attrs)
+{
+	*fmt_attr = kcalloc(fmt_num_attrs, sizeof(**fmt_attr), GFP_KERNEL);
 
-	if (!pmu_entry)
+	if (!(*fmt_attr))
 		return -ENOMEM;
 
-	pmu_entry->adev = adev;
+	fmt_attr_group->attrs = kcalloc(fmt_num_attrs + 1,
+				sizeof(*fmt_attr_group->attrs), GFP_KERNEL);
+
+	if (!fmt_attr_group->attrs)
+		goto err_fmt_attr_grp;
+
+	*evt_attr = kcalloc(evt_num_attrs, sizeof(**evt_attr), GFP_KERNEL);
+
+	if (!(*evt_attr))
+		goto err_evt_attr;
+
+	evt_attr_group->attrs = kcalloc(evt_num_attrs + 1,
+				sizeof(*evt_attr_group->attrs), GFP_KERNEL);
+
+	if (!evt_attr_group->attrs)
+		goto err_evt_attr_grp;
+
+	return 0;
+err_evt_attr_grp:
+	kfree(*evt_attr);
+err_evt_attr:
+	kfree(fmt_attr_group->attrs);
+err_fmt_attr_grp:
+	kfree(*fmt_attr);
+	return -ENOMEM;
+}
+
+/* init pmu tracking per pmu type */
+static int init_pmu_by_type(struct amdgpu_pmu_entry *pmu_entry,
+			char *pmu_type_name, char *pmu_file_prefix,
+			unsigned int pmu_perf_type)

I feel this function could be a simpler ASIC independent helper function. If you check all asic_types instead of just VEGA20 in amdgpu_pmu_init(), this function could be something like this  
init_pmu_by_type(struct amdgpu_pmu_entry *pmu_entry,
			char *pmu_type_name, char *pmu_file_prefix,
			unsigned int pmu_perf_type,
			const char *events[][2], int num_of_events,
			const char *fmts[][2], int mum_of_events);
This way you could probably avoid all "is_legacy ?" cases


+{
+	const struct attribute_group *attr_groups[] = {
+		&pmu_entry->fmt_attr_group,
+		&pmu_entry->evt_attr_group,
+		NULL
+	};
+	char pmu_name[PMU_NAME_SIZE];
+	bool is_legacy = pmu_perf_type == PERF_TYPE_AMDGPU_DF_LEGACY;
+	int ret = 0, num_events = 0;
+
 	pmu_entry->pmu = (struct pmu){
 		.event_init = amdgpu_perf_event_init,
 		.add = amdgpu_perf_add,
@@ -237,59 +378,157 @@ static int init_pmu_by_type(struct amdgpu_device *adev,
 		.task_ctx_nr = perf_invalid_context,
 	};
 
-	pmu_entry->pmu.attr_groups = attr_groups;
+	switch (pmu_entry->adev->asic_type) {
+	case CHIP_VEGA20:
+		ret = amdgpu_pmu_alloc_pmu_attrs(&pmu_entry->fmt_attr_group,
+					&pmu_entry->fmt_attr,
+					is_legacy ? NUM_FORMATS_DF_LEGACY :
+							NUM_FORMATS_AMDGPU_PMU,
+					&pmu_entry->evt_attr_group,
+					&pmu_entry->evt_attr,
+					is_legacy ? NUM_EVENTS_DF_LEGACY :
+							NUM_EVENTS_VEGA20_MAX);
+
+		if (ret)
+			goto err_out;
+
+		amdgpu_pmu_create_attributes(&pmu_entry->fmt_attr_group,
+					pmu_entry->fmt_attr,
+					is_legacy ? df_vega20_formats :
+							amdgpu_pmu_formats, 0,
+					is_legacy ? NUM_FORMATS_DF_LEGACY :
+							NUM_FORMATS_AMDGPU_PMU,
+					0);
+
+		amdgpu_pmu_create_attributes(&pmu_entry->evt_attr_group,
+					pmu_entry->evt_attr,
+					is_legacy ? df_vega20_events :
+							vega20_events, 0,
+					is_legacy ? NUM_EVENTS_DF_LEGACY :
+							NUM_EVENTS_VEGA20_XGMI,
+					is_legacy ? PERF_TYPE_AMDGPU_DF_LEGACY :
+							PERF_TYPE_AMDGPU_XGMI);
+		num_events += is_legacy ? NUM_EVENTS_DF_LEGACY :
+							NUM_EVENTS_VEGA20_XGMI;
+

Are you using += here in preparation for future events. Because this variable num_events is used only once in this function

+		/* other events can be added here */
+
+		break;
+	default:
+		ret = -ENODEV;
+		goto err_out;
+	};
+
+	pmu_entry->pmu.attr_groups = kmemdup(attr_groups, sizeof(attr_groups),
+								GFP_KERNEL);
+
+	if (!pmu_entry->pmu.attr_groups)
+		goto err_attr_group;
+
 	pmu_entry->pmu_perf_type = pmu_perf_type;
-	snprintf(pmu_name, PMU_NAME_SIZE, "%s_%d",
-				pmu_file_prefix, adev_to_drm(adev)->primary->index);
+	snprintf(pmu_name, PMU_NAME_SIZE, "%s_%d", pmu_file_prefix,
+				adev_to_drm(pmu_entry->adev)->primary->index);
 
 	ret = perf_pmu_register(&pmu_entry->pmu, pmu_name, -1);
 
-	if (ret) {
-		kfree(pmu_entry);
-		pr_warn("Error initializing AMDGPU %s PMUs.\n", pmu_type_name);
-		return ret;
-	}
+	if (ret)
+		goto err_register;
 
 	pr_info("Detected AMDGPU %s Counters. # of Counters = %d.\n",
-			pmu_type_name, num_counters);
+			pmu_type_name, num_events);

Should the "# of Counters" be changes to "# of Events"

 	list_add_tail(&pmu_entry->entry, &amdgpu_pmu_list);
 
 	return 0;
+err_register:
+	kfree(pmu_entry->pmu.attr_groups);
+err_attr_group:
+	kfree(pmu_entry->fmt_attr_group.attrs);
+	kfree(pmu_entry->fmt_attr);
+	kfree(pmu_entry->evt_attr_group.attrs);
+	kfree(pmu_entry->evt_attr);
+err_out:
+	pr_warn("Error initializing AMDGPU %s PMUs.\n", pmu_type_name);
+	return ret;
+}
+
+/* destroy all pmu data associated with target device */
+void amdgpu_pmu_fini(struct amdgpu_device *adev)
+{
+	struct amdgpu_pmu_entry *pe, *temp;
+
+	list_for_each_entry_safe(pe, temp, &amdgpu_pmu_list, entry) {
+		if (pe->adev != adev)
+			continue;
+		list_del(&pe->entry);
+		perf_pmu_unregister(&pe->pmu);
+		kfree(pe->pmu.attr_groups);
+		kfree(pe->fmt_attr_group.attrs);
+		kfree(pe->fmt_attr);
+		kfree(pe->evt_attr_group.attrs);
+		kfree(pe->evt_attr);
+		kfree(pe);
+	}
+}
+
+static bool amdgpu_pmu_is_supported(struct amdgpu_device *adev)
+{
+	return adev->asic_type == CHIP_VEGA20;
 }
 
 /* init amdgpu_pmu */
 int amdgpu_pmu_init(struct amdgpu_device *adev)
 {
 	int ret = 0;
+	struct amdgpu_pmu_entry *pmu_entry, *pmu_entry_legacy;
 
-	switch (adev->asic_type) {
-	case CHIP_VEGA20:
-		/* init df */
-		ret = init_pmu_by_type(adev, df_v3_6_attr_groups,
-				       "DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF,
-				       DF_V3_6_MAX_COUNTERS);
-
-		/* other pmu types go here*/
-		break;
-	default:
+	if (!amdgpu_pmu_is_supported(adev))
 		return 0;
-	}
 
-	return 0;
-}
+	if (adev->asic_type == CHIP_VEGA20) {
+		pmu_entry_legacy = kzalloc(sizeof(struct amdgpu_pmu_entry),
+								GFP_KERNEL);
 
+		if (!pmu_entry_legacy)
+			return -ENOMEM;
 
-/* destroy all pmu data associated with target device */
-void amdgpu_pmu_fini(struct amdgpu_device *adev)
-{
-	struct amdgpu_pmu_entry *pe, *temp;
+		pmu_entry_legacy->adev = adev;
+		pmu_entry_legacy->fmt_attr_group.name = "format";
+		pmu_entry_legacy->fmt_attr_group.attrs = NULL;
+		pmu_entry_legacy->evt_attr_group.name = "events";
+		pmu_entry_legacy->evt_attr_group.attrs = NULL;
 
-	list_for_each_entry_safe(pe, temp, &amdgpu_pmu_list, entry) {
-		if (pe->adev == adev) {
-			list_del(&pe->entry);
-			perf_pmu_unregister(&pe->pmu);
-			kfree(pe);
+		ret = init_pmu_by_type(pmu_entry_legacy, "DF", "amdgpu_df",
+						PERF_TYPE_AMDGPU_DF_LEGACY);
+
+		if (ret) {
+			kfree(pmu_entry_legacy);
+			return ret;
 		}
 	}
+
+	pmu_entry = kzalloc(sizeof(struct amdgpu_pmu_entry), GFP_KERNEL);
+
+	if (!pmu_entry) {
+		if (adev->asic_type == CHIP_VEGA20)
+			amdgpu_pmu_fini(adev);
+		return -ENOMEM;
+	}
+
+	pmu_entry->adev = adev;
+	pmu_entry->fmt_attr_group.name = "format";
+	pmu_entry->fmt_attr_group.attrs = NULL;
+	pmu_entry->evt_attr_group.name = "events";
+	pmu_entry->evt_attr_group.attrs = NULL;
+
+	ret = init_pmu_by_type(pmu_entry, "Event", "amdgpu",
+							PERF_TYPE_AMDGPU_MAX);
+	if (ret) {
+		if (adev->asic_type == CHIP_VEGA20)
+			amdgpu_pmu_fini(adev);
+		kfree(pmu_entry);
+
+	}
+
+	return ret;
 }
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.h
index 7dddb7160a11..0d214abe720e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.h
@@ -27,10 +27,14 @@
 #define _AMDGPU_PMU_H_
 
 enum amdgpu_pmu_perf_type {
-	PERF_TYPE_AMDGPU_DF = 0,
+	PERF_TYPE_AMDGPU_DF_LEGACY = 0,
+	PERF_TYPE_AMDGPU_XGMI,
 	PERF_TYPE_AMDGPU_MAX
 };
 
+#define AMDGPU_PERF_TYPE_SHIFT	56
+#define AMDGPU_PERF_TYPE_MASK	0xff
+
 int amdgpu_pmu_init(struct amdgpu_device *adev);
 void amdgpu_pmu_fini(struct amdgpu_device *adev);
 
diff --git a/drivers/gpu/drm/amd/amdgpu/df_v3_6.c b/drivers/gpu/drm/amd/amdgpu/df_v3_6.c
index 0ca6e176acb0..6e57ae95f997 100644
--- a/drivers/gpu/drm/amd/amdgpu/df_v3_6.c
+++ b/drivers/gpu/drm/amd/amdgpu/df_v3_6.c
@@ -30,71 +30,17 @@
 #define DF_3_6_SMN_REG_INST_DIST        0x8
 #define DF_3_6_INST_CNT                 8
 
-static u32 df_v3_6_channel_number[] = {1, 2, 0, 4, 0, 8, 0,
-				       16, 32, 0, 0, 0, 2, 4, 8};
-
-/* init df format attrs */
-AMDGPU_PMU_ATTR(event,		"config:0-7");
-AMDGPU_PMU_ATTR(instance,	"config:8-15");
-AMDGPU_PMU_ATTR(umask,		"config:16-23");
-
-/* df format attributes  */
-static struct attribute *df_v3_6_format_attrs[] = {
-	&pmu_attr_event.attr,
-	&pmu_attr_instance.attr,
-	&pmu_attr_umask.attr,
-	NULL
-};
-
-/* df format attribute group */
-static struct attribute_group df_v3_6_format_attr_group = {
-	.name = "format",
-	.attrs = df_v3_6_format_attrs,
-};
+/* Defined in global_features.h as FTI_PERFMON_VISIBLE */
+#define DF_V3_6_MAX_COUNTERS		4
 
-/* df event attrs */
-AMDGPU_PMU_ATTR(cake0_pcsout_txdata,
-		      "event=0x7,instance=0x46,umask=0x2");
-AMDGPU_PMU_ATTR(cake1_pcsout_txdata,
-		      "event=0x7,instance=0x47,umask=0x2");
-AMDGPU_PMU_ATTR(cake0_pcsout_txmeta,
-		      "event=0x7,instance=0x46,umask=0x4");
-AMDGPU_PMU_ATTR(cake1_pcsout_txmeta,
-		      "event=0x7,instance=0x47,umask=0x4");
-AMDGPU_PMU_ATTR(cake0_ftiinstat_reqalloc,
-		      "event=0xb,instance=0x46,umask=0x4");
-AMDGPU_PMU_ATTR(cake1_ftiinstat_reqalloc,
-		      "event=0xb,instance=0x47,umask=0x4");
-AMDGPU_PMU_ATTR(cake0_ftiinstat_rspalloc,
-		      "event=0xb,instance=0x46,umask=0x8");
-AMDGPU_PMU_ATTR(cake1_ftiinstat_rspalloc,
-		      "event=0xb,instance=0x47,umask=0x8");
-
-/* df event attributes  */
-static struct attribute *df_v3_6_event_attrs[] = {
-	&pmu_attr_cake0_pcsout_txdata.attr,
-	&pmu_attr_cake1_pcsout_txdata.attr,
-	&pmu_attr_cake0_pcsout_txmeta.attr,
-	&pmu_attr_cake1_pcsout_txmeta.attr,
-	&pmu_attr_cake0_ftiinstat_reqalloc.attr,
-	&pmu_attr_cake1_ftiinstat_reqalloc.attr,
-	&pmu_attr_cake0_ftiinstat_rspalloc.attr,
-	&pmu_attr_cake1_ftiinstat_rspalloc.attr,
-	NULL
-};
-
-/* df event attribute group */
-static struct attribute_group df_v3_6_event_attr_group = {
-	.name = "events",
-	.attrs = df_v3_6_event_attrs
-};
+/* get flags from df perfmon config */
+#define DF_V3_6_GET_EVENT(x)		(x & 0xFFUL)
+#define DF_V3_6_GET_INSTANCE(x)		((x >> 8) & 0xFFUL)
+#define DF_V3_6_GET_UNITMASK(x)		((x >> 16) & 0xFFUL)
+#define DF_V3_6_PERFMON_OVERFLOW	0xFFFFFFFFFFFFULL
 
-/* df event attr groups  */
-const struct attribute_group *df_v3_6_attr_groups[] = {
-		&df_v3_6_format_attr_group,
-		&df_v3_6_event_attr_group,
-		NULL
-};
+static u32 df_v3_6_channel_number[] = {1, 2, 0, 4, 0, 8, 0,
+				       16, 32, 0, 0, 0, 2, 4, 8};
 
 static uint64_t df_v3_6_get_fica(struct amdgpu_device *adev,
 				 uint32_t ficaa_val)
diff --git a/drivers/gpu/drm/amd/amdgpu/df_v3_6.h b/drivers/gpu/drm/amd/amdgpu/df_v3_6.h
index 76998541bc30..2505c7ef258a 100644
--- a/drivers/gpu/drm/amd/amdgpu/df_v3_6.h
+++ b/drivers/gpu/drm/amd/amdgpu/df_v3_6.h
@@ -35,15 +35,6 @@ enum DF_V3_6_MGCG {
 	DF_V3_6_MGCG_ENABLE_63_CYCLE_DELAY = 15
 };
 
-/* Defined in global_features.h as FTI_PERFMON_VISIBLE */
-#define DF_V3_6_MAX_COUNTERS		4
-
-/* get flags from df perfmon config */
-#define DF_V3_6_GET_EVENT(x)		(x & 0xFFUL)
-#define DF_V3_6_GET_INSTANCE(x)		((x >> 8) & 0xFFUL)
-#define DF_V3_6_GET_UNITMASK(x)		((x >> 16) & 0xFFUL)
-#define DF_V3_6_PERFMON_OVERFLOW	0xFFFFFFFFFFFFULL
-
 extern const struct attribute_group *df_v3_6_attr_groups[];
 extern const struct amdgpu_df_funcs df_v3_6_funcs;
 
-- 
2.17.1
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 2/3] drm/amdgpu: add per device user friendly xgmi events for vega20

[Jonathan Kim]

Non-outbound data metrics are non useful so mark them as legacy.
Bucket new perf counters into device and not device ip.
Bind events to chip instead of IP.
Report available event counters and not number of hw counter banks.
Move DF public macros to private since not needed outside of IP version.

v3: attr groups const array is global but attr groups are allocated per
device which doesn't work and causes problems on memory allocation and
de-allocation for pmu unregister. Switch to building const attr groups
per pmu instead to simplify solution.

v2: add comments on sysfs structure and formatting.

Signed-off-by: Jonathan Kim <Jonathan.Kim@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h     |  13 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c | 341 ++++++++++++++++++++----
 drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.h |   6 +-
 drivers/gpu/drm/amd/amdgpu/df_v3_6.c    |  72 +----
 drivers/gpu/drm/amd/amdgpu/df_v3_6.h    |   9 -
 5 files changed, 304 insertions(+), 137 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 13f92dea182a..f43dfdd2716a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1279,19 +1279,6 @@ bool amdgpu_device_load_pci_state(struct pci_dev *pdev);
 
 #include "amdgpu_object.h"
 
-/* used by df_v3_6.c and amdgpu_pmu.c */
-#define AMDGPU_PMU_ATTR(_name, _object)					\
-static ssize_t								\
-_name##_show(struct device *dev,					\
-			       struct device_attribute *attr,		\
-			       char *page)				\
-{									\
-	BUILD_BUG_ON(sizeof(_object) >= PAGE_SIZE - 1);			\
-	return sprintf(page, _object "\n");				\
-}									\
-									\
-static struct device_attribute pmu_attr_##_name = __ATTR_RO(_name)
-
 static inline bool amdgpu_is_tmz(struct amdgpu_device *adev)
 {
        return adev->gmc.tmz_enabled;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
index 1b0ec715c8ba..74fe8fbdc0d1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
@@ -27,9 +27,19 @@
 #include <linux/init.h>
 #include "amdgpu.h"
 #include "amdgpu_pmu.h"
-#include "df_v3_6.h"
 
 #define PMU_NAME_SIZE 32
+#define NUM_FORMATS_AMDGPU_PMU		4
+#define NUM_FORMATS_DF_LEGACY		3
+#define NUM_EVENTS_DF_LEGACY		8
+#define NUM_EVENTS_VEGA20_XGMI		2
+#define NUM_EVENTS_VEGA20_MAX		NUM_EVENTS_VEGA20_XGMI
+
+struct amdgpu_pmu_event_attribute {
+	struct device_attribute attr;
+	const char *event_str;
+	unsigned int type;
+};
 
 /* record to keep track of pmu entry per pmu type per device */
 struct amdgpu_pmu_entry {
@@ -37,10 +47,74 @@ struct amdgpu_pmu_entry {
 	struct amdgpu_device *adev;
 	struct pmu pmu;
 	unsigned int pmu_perf_type;
+	struct attribute_group fmt_attr_group;
+	struct amdgpu_pmu_event_attribute *fmt_attr;
+	struct attribute_group evt_attr_group;
+	struct amdgpu_pmu_event_attribute *evt_attr;
 };
 
+static ssize_t amdgpu_pmu_event_show(struct device *dev,
+				struct device_attribute *attr, char *buf)
+{
+	struct amdgpu_pmu_event_attribute *amdgpu_pmu_attr;
+
+	amdgpu_pmu_attr = container_of(attr, struct amdgpu_pmu_event_attribute,
+									attr);
+
+	if (!amdgpu_pmu_attr->type)
+		return sprintf(buf, "%s\n", amdgpu_pmu_attr->event_str);
+
+	return sprintf(buf, "%s,type=0x%x\n",
+			amdgpu_pmu_attr->event_str, amdgpu_pmu_attr->type);
+}
+
 static LIST_HEAD(amdgpu_pmu_list);
 
+/*
+ * Event formatting is global to all amdgpu events under sysfs folder
+ * /sys/bus/event_source/devices/amdgpu_<dev_num> where dev_num is the
+ * primary device index. Registered events can be found in subfolder "events"
+ * and formatting under subfolder "format".
+ *
+ * Formats "event", "instance", and "umask" are currently used by xGMI but can
+ * be for generalized for other IP usage.  If format naming is insufficient
+ * for newly registered IP events, append to the list below and handle the
+ * perf events hardware configuration (see hwc->config) as required by the IP.
+ *
+ * Format "type" indicates IP type generated on pmu registration (see
+ * init_pmu_by_type) so non-legacy events omit this in the per-chip event
+ * list (e.g. vega20_events).
+ */
+static const char *amdgpu_pmu_formats[NUM_FORMATS_AMDGPU_PMU][2] = {
+	{ "event", "config:0-7" },
+	{ "instance", "config:8-15" },
+	{ "umask", "config:16-23"},
+	{ "type", "config:56-63"}
+};
+
+/* Vega20 events */
+static const char *vega20_events[NUM_EVENTS_VEGA20_MAX][2] = {
+	{ "xgmi_link0_data_outbound", "event=0x7,instance=0x46,umask=0x2" },
+	{ "xgmi_link1_data_outbound", "event=0x7,instance=0x47,umask=0x2" }
+};
+
+/* All df_vega20_* items are DEPRECATED. Use vega20_ items above instead. */
+static const char *df_vega20_formats[NUM_FORMATS_DF_LEGACY][2] = {
+	{ "event", "config:0-7" },
+	{ "instance", "config:8-15" },
+	{ "umask", "config:16-23"}
+};
+
+static const char *df_vega20_events[NUM_EVENTS_DF_LEGACY][2] = {
+	{ "cake0_pcsout_txdata", "event=0x7,instance=0x46,umask=0x2" },
+	{ "cake1_pcsout_txdata", "event=0x7,instance=0x47,umask=0x2" },
+	{ "cake0_pcsout_txmeta", "event=0x7,instance=0x46,umask=0x4" },
+	{ "cake1_pcsout_txmeta", "event=0x7,instance=0x47,umask=0x4" },
+	{ "cake0_ftiinstat_reqalloc", "event=0xb,instance=0x46,umask=0x4" },
+	{ "cake1_ftiinstat_reqalloc", "event=0xb,instance=0x47,umask=0x4" },
+	{ "cake0_ftiinstat_rspalloc", "event=0xb,instance=0x46,umask=0x8" },
+	{ "cake1_ftiinstat_rspalloc", "event=0xb,instance=0x47,umask=0x8" },
+};
 
 /* initialize perf counter */
 static int amdgpu_perf_event_init(struct perf_event *event)
@@ -73,7 +147,8 @@ static void amdgpu_perf_start(struct perf_event *event, int flags)
 	hwc->state = 0;
 
 	switch (pe->pmu_perf_type) {
-	case PERF_TYPE_AMDGPU_DF:
+	case PERF_TYPE_AMDGPU_DF_LEGACY:
+	case PERF_TYPE_AMDGPU_XGMI:
 		if (!(flags & PERF_EF_RELOAD)) {
 			target_cntr = pe->adev->df.funcs->pmc_start(pe->adev,
 						hwc->config, 0 /* unused */,
@@ -108,7 +183,8 @@ static void amdgpu_perf_read(struct perf_event *event)
 		prev = local64_read(&hwc->prev_count);
 
 		switch (pe->pmu_perf_type) {
-		case PERF_TYPE_AMDGPU_DF:
+		case PERF_TYPE_AMDGPU_DF_LEGACY:
+		case PERF_TYPE_AMDGPU_XGMI:
 			pe->adev->df.funcs->pmc_get_count(pe->adev,
 						hwc->config, hwc->idx, &count);
 			break;
@@ -133,7 +209,8 @@ static void amdgpu_perf_stop(struct perf_event *event, int flags)
 		return;
 
 	switch (pe->pmu_perf_type) {
-	case PERF_TYPE_AMDGPU_DF:
+	case PERF_TYPE_AMDGPU_DF_LEGACY:
+	case PERF_TYPE_AMDGPU_XGMI:
 		pe->adev->df.funcs->pmc_stop(pe->adev, hwc->config, hwc->idx,
 									0);
 		break;
@@ -160,10 +237,15 @@ static int amdgpu_perf_add(struct perf_event *event, int flags)
 						  struct amdgpu_pmu_entry,
 						  pmu);
 
+	if (pe->pmu_perf_type == PERF_TYPE_AMDGPU_MAX)
+		pe->pmu_perf_type = (hwc->config >> AMDGPU_PERF_TYPE_SHIFT) &
+							AMDGPU_PERF_TYPE_MASK;
+
 	event->hw.state = PERF_HES_UPTODATE | PERF_HES_STOPPED;
 
 	switch (pe->pmu_perf_type) {
-	case PERF_TYPE_AMDGPU_DF:
+	case PERF_TYPE_AMDGPU_DF_LEGACY:
+	case PERF_TYPE_AMDGPU_XGMI:
 		target_cntr = pe->adev->df.funcs->pmc_start(pe->adev,
 						hwc->config, 0 /* unused */,
 						1 /* add counter */);
@@ -197,7 +279,8 @@ static void amdgpu_perf_del(struct perf_event *event, int flags)
 	amdgpu_perf_stop(event, PERF_EF_UPDATE);
 
 	switch (pe->pmu_perf_type) {
-	case PERF_TYPE_AMDGPU_DF:
+	case PERF_TYPE_AMDGPU_DF_LEGACY:
+	case PERF_TYPE_AMDGPU_XGMI:
 		pe->adev->df.funcs->pmc_stop(pe->adev, hwc->config, hwc->idx,
 									1);
 		break;
@@ -208,25 +291,83 @@ static void amdgpu_perf_del(struct perf_event *event, int flags)
 	perf_event_update_userpage(event);
 }
 
-/* vega20 pmus */
-
-/* init pmu tracking per pmu type */
-static int init_pmu_by_type(struct amdgpu_device *adev,
-		  const struct attribute_group *attr_groups[],
-		  char *pmu_type_name, char *pmu_file_prefix,
-		  unsigned int pmu_perf_type,
-		  unsigned int num_counters)
+static void amdgpu_pmu_create_attributes(struct attribute_group *attr_group,
+				struct amdgpu_pmu_event_attribute *pmu_attr,
+				const char *events[][2],
+				int s_offset,
+				int e_offset,
+				unsigned int type)
 {
-	char pmu_name[PMU_NAME_SIZE];
-	struct amdgpu_pmu_entry *pmu_entry;
-	int ret = 0;
+	int i;
+
+	pmu_attr += s_offset;
+
+	for (i = s_offset; i < e_offset; i++) {
+		attr_group->attrs[i] = &pmu_attr->attr.attr;
+		sysfs_attr_init(&pmu_attr->attr.attr);
+		pmu_attr->attr.attr.name = events[i][0];
+		pmu_attr->attr.attr.mode = 0444;
+		pmu_attr->attr.show = amdgpu_pmu_event_show;
+		pmu_attr->event_str = events[i][1];
+		pmu_attr->type = type;
+		pmu_attr++;
+	}
+}
 
-	pmu_entry = kzalloc(sizeof(struct amdgpu_pmu_entry), GFP_KERNEL);
+static int amdgpu_pmu_alloc_pmu_attrs(
+				struct attribute_group *fmt_attr_group,
+				struct amdgpu_pmu_event_attribute **fmt_attr,
+				int fmt_num_attrs,
+				struct attribute_group *evt_attr_group,
+				struct amdgpu_pmu_event_attribute **evt_attr,
+				int evt_num_attrs)
+{
+	*fmt_attr = kcalloc(fmt_num_attrs, sizeof(**fmt_attr), GFP_KERNEL);
 
-	if (!pmu_entry)
+	if (!(*fmt_attr))
 		return -ENOMEM;
 
-	pmu_entry->adev = adev;
+	fmt_attr_group->attrs = kcalloc(fmt_num_attrs + 1,
+				sizeof(*fmt_attr_group->attrs), GFP_KERNEL);
+
+	if (!fmt_attr_group->attrs)
+		goto err_fmt_attr_grp;
+
+	*evt_attr = kcalloc(evt_num_attrs, sizeof(**evt_attr), GFP_KERNEL);
+
+	if (!(*evt_attr))
+		goto err_evt_attr;
+
+	evt_attr_group->attrs = kcalloc(evt_num_attrs + 1,
+				sizeof(*evt_attr_group->attrs), GFP_KERNEL);
+
+	if (!evt_attr_group->attrs)
+		goto err_evt_attr_grp;
+
+	return 0;
+err_evt_attr_grp:
+	kfree(*evt_attr);
+err_evt_attr:
+	kfree(fmt_attr_group->attrs);
+err_fmt_attr_grp:
+	kfree(*fmt_attr);
+	return -ENOMEM;
+}
+
+/* init pmu tracking per pmu type */
+static int init_pmu_by_type(struct amdgpu_pmu_entry *pmu_entry,
+			char *pmu_type_name, char *pmu_file_prefix,
+			unsigned int pmu_perf_type)
+{
+	const struct attribute_group *attr_groups[] = {
+		&pmu_entry->fmt_attr_group,
+		&pmu_entry->evt_attr_group,
+		NULL
+	};
+	char pmu_name[PMU_NAME_SIZE];
+	bool is_legacy = pmu_perf_type == PERF_TYPE_AMDGPU_DF_LEGACY;
+	int ret = 0, num_events = 0;
+
 	pmu_entry->pmu = (struct pmu){
 		.event_init = amdgpu_perf_event_init,
 		.add = amdgpu_perf_add,
@@ -237,59 +378,157 @@ static int init_pmu_by_type(struct amdgpu_device *adev,
 		.task_ctx_nr = perf_invalid_context,
 	};
 
-	pmu_entry->pmu.attr_groups = attr_groups;
+	switch (pmu_entry->adev->asic_type) {
+	case CHIP_VEGA20:
+		ret = amdgpu_pmu_alloc_pmu_attrs(&pmu_entry->fmt_attr_group,
+					&pmu_entry->fmt_attr,
+					is_legacy ? NUM_FORMATS_DF_LEGACY :
+							NUM_FORMATS_AMDGPU_PMU,
+					&pmu_entry->evt_attr_group,
+					&pmu_entry->evt_attr,
+					is_legacy ? NUM_EVENTS_DF_LEGACY :
+							NUM_EVENTS_VEGA20_MAX);
+
+		if (ret)
+			goto err_out;
+
+		amdgpu_pmu_create_attributes(&pmu_entry->fmt_attr_group,
+					pmu_entry->fmt_attr,
+					is_legacy ? df_vega20_formats :
+							amdgpu_pmu_formats, 0,
+					is_legacy ? NUM_FORMATS_DF_LEGACY :
+							NUM_FORMATS_AMDGPU_PMU,
+					0);
+
+		amdgpu_pmu_create_attributes(&pmu_entry->evt_attr_group,
+					pmu_entry->evt_attr,
+					is_legacy ? df_vega20_events :
+							vega20_events, 0,
+					is_legacy ? NUM_EVENTS_DF_LEGACY :
+							NUM_EVENTS_VEGA20_XGMI,
+					is_legacy ? PERF_TYPE_AMDGPU_DF_LEGACY :
+							PERF_TYPE_AMDGPU_XGMI);
+		num_events += is_legacy ? NUM_EVENTS_DF_LEGACY :
+							NUM_EVENTS_VEGA20_XGMI;
+
+		/* other events can be added here */
+
+		break;
+	default:
+		ret = -ENODEV;
+		goto err_out;
+	};
+
+	pmu_entry->pmu.attr_groups = kmemdup(attr_groups, sizeof(attr_groups),
+								GFP_KERNEL);
+
+	if (!pmu_entry->pmu.attr_groups)
+		goto err_attr_group;
+
 	pmu_entry->pmu_perf_type = pmu_perf_type;
-	snprintf(pmu_name, PMU_NAME_SIZE, "%s_%d",
-				pmu_file_prefix, adev_to_drm(adev)->primary->index);
+	snprintf(pmu_name, PMU_NAME_SIZE, "%s_%d", pmu_file_prefix,
+				adev_to_drm(pmu_entry->adev)->primary->index);
 
 	ret = perf_pmu_register(&pmu_entry->pmu, pmu_name, -1);
 
-	if (ret) {
-		kfree(pmu_entry);
-		pr_warn("Error initializing AMDGPU %s PMUs.\n", pmu_type_name);
-		return ret;
-	}
+	if (ret)
+		goto err_register;
 
 	pr_info("Detected AMDGPU %s Counters. # of Counters = %d.\n",
-			pmu_type_name, num_counters);
+			pmu_type_name, num_events);
 
 	list_add_tail(&pmu_entry->entry, &amdgpu_pmu_list);
 
 	return 0;
+err_register:
+	kfree(pmu_entry->pmu.attr_groups);
+err_attr_group:
+	kfree(pmu_entry->fmt_attr_group.attrs);
+	kfree(pmu_entry->fmt_attr);
+	kfree(pmu_entry->evt_attr_group.attrs);
+	kfree(pmu_entry->evt_attr);
+err_out:
+	pr_warn("Error initializing AMDGPU %s PMUs.\n", pmu_type_name);
+	return ret;
+}
+
+/* destroy all pmu data associated with target device */
+void amdgpu_pmu_fini(struct amdgpu_device *adev)
+{
+	struct amdgpu_pmu_entry *pe, *temp;
+
+	list_for_each_entry_safe(pe, temp, &amdgpu_pmu_list, entry) {
+		if (pe->adev != adev)
+			continue;
+		list_del(&pe->entry);
+		perf_pmu_unregister(&pe->pmu);
+		kfree(pe->pmu.attr_groups);
+		kfree(pe->fmt_attr_group.attrs);
+		kfree(pe->fmt_attr);
+		kfree(pe->evt_attr_group.attrs);
+		kfree(pe->evt_attr);
+		kfree(pe);
+	}
+}
+
+static bool amdgpu_pmu_is_supported(struct amdgpu_device *adev)
+{
+	return adev->asic_type == CHIP_VEGA20;
 }
 
 /* init amdgpu_pmu */
 int amdgpu_pmu_init(struct amdgpu_device *adev)
 {
 	int ret = 0;
+	struct amdgpu_pmu_entry *pmu_entry, *pmu_entry_legacy;
 
-	switch (adev->asic_type) {
-	case CHIP_VEGA20:
-		/* init df */
-		ret = init_pmu_by_type(adev, df_v3_6_attr_groups,
-				       "DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF,
-				       DF_V3_6_MAX_COUNTERS);
-
-		/* other pmu types go here*/
-		break;
-	default:
+	if (!amdgpu_pmu_is_supported(adev))
 		return 0;
-	}
 
-	return 0;
-}
+	if (adev->asic_type == CHIP_VEGA20) {
+		pmu_entry_legacy = kzalloc(sizeof(struct amdgpu_pmu_entry),
+								GFP_KERNEL);
 
+		if (!pmu_entry_legacy)
+			return -ENOMEM;
 
-/* destroy all pmu data associated with target device */
-void amdgpu_pmu_fini(struct amdgpu_device *adev)
-{
-	struct amdgpu_pmu_entry *pe, *temp;
+		pmu_entry_legacy->adev = adev;
+		pmu_entry_legacy->fmt_attr_group.name = "format";
+		pmu_entry_legacy->fmt_attr_group.attrs = NULL;
+		pmu_entry_legacy->evt_attr_group.name = "events";
+		pmu_entry_legacy->evt_attr_group.attrs = NULL;
 
-	list_for_each_entry_safe(pe, temp, &amdgpu_pmu_list, entry) {
-		if (pe->adev == adev) {
-			list_del(&pe->entry);
-			perf_pmu_unregister(&pe->pmu);
-			kfree(pe);
+		ret = init_pmu_by_type(pmu_entry_legacy, "DF", "amdgpu_df",
+						PERF_TYPE_AMDGPU_DF_LEGACY);
+
+		if (ret) {
+			kfree(pmu_entry_legacy);
+			return ret;
 		}
 	}
+
+	pmu_entry = kzalloc(sizeof(struct amdgpu_pmu_entry), GFP_KERNEL);
+
+	if (!pmu_entry) {
+		if (adev->asic_type == CHIP_VEGA20)
+			amdgpu_pmu_fini(adev);
+		return -ENOMEM;
+	}
+
+	pmu_entry->adev = adev;
+	pmu_entry->fmt_attr_group.name = "format";
+	pmu_entry->fmt_attr_group.attrs = NULL;
+	pmu_entry->evt_attr_group.name = "events";
+	pmu_entry->evt_attr_group.attrs = NULL;
+
+	ret = init_pmu_by_type(pmu_entry, "Event", "amdgpu",
+							PERF_TYPE_AMDGPU_MAX);
+	if (ret) {
+		if (adev->asic_type == CHIP_VEGA20)
+			amdgpu_pmu_fini(adev);
+		kfree(pmu_entry);
+
+	}
+
+	return ret;
 }
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.h
index 7dddb7160a11..0d214abe720e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.h
@@ -27,10 +27,14 @@
 #define _AMDGPU_PMU_H_
 
 enum amdgpu_pmu_perf_type {
-	PERF_TYPE_AMDGPU_DF = 0,
+	PERF_TYPE_AMDGPU_DF_LEGACY = 0,
+	PERF_TYPE_AMDGPU_XGMI,
 	PERF_TYPE_AMDGPU_MAX
 };
 
+#define AMDGPU_PERF_TYPE_SHIFT	56
+#define AMDGPU_PERF_TYPE_MASK	0xff
+
 int amdgpu_pmu_init(struct amdgpu_device *adev);
 void amdgpu_pmu_fini(struct amdgpu_device *adev);
 
diff --git a/drivers/gpu/drm/amd/amdgpu/df_v3_6.c b/drivers/gpu/drm/amd/amdgpu/df_v3_6.c
index 0ca6e176acb0..6e57ae95f997 100644
--- a/drivers/gpu/drm/amd/amdgpu/df_v3_6.c
+++ b/drivers/gpu/drm/amd/amdgpu/df_v3_6.c
@@ -30,71 +30,17 @@
 #define DF_3_6_SMN_REG_INST_DIST        0x8
 #define DF_3_6_INST_CNT                 8
 
-static u32 df_v3_6_channel_number[] = {1, 2, 0, 4, 0, 8, 0,
-				       16, 32, 0, 0, 0, 2, 4, 8};
-
-/* init df format attrs */
-AMDGPU_PMU_ATTR(event,		"config:0-7");
-AMDGPU_PMU_ATTR(instance,	"config:8-15");
-AMDGPU_PMU_ATTR(umask,		"config:16-23");
-
-/* df format attributes  */
-static struct attribute *df_v3_6_format_attrs[] = {
-	&pmu_attr_event.attr,
-	&pmu_attr_instance.attr,
-	&pmu_attr_umask.attr,
-	NULL
-};
-
-/* df format attribute group */
-static struct attribute_group df_v3_6_format_attr_group = {
-	.name = "format",
-	.attrs = df_v3_6_format_attrs,
-};
+/* Defined in global_features.h as FTI_PERFMON_VISIBLE */
+#define DF_V3_6_MAX_COUNTERS		4
 
-/* df event attrs */
-AMDGPU_PMU_ATTR(cake0_pcsout_txdata,
-		      "event=0x7,instance=0x46,umask=0x2");
-AMDGPU_PMU_ATTR(cake1_pcsout_txdata,
-		      "event=0x7,instance=0x47,umask=0x2");
-AMDGPU_PMU_ATTR(cake0_pcsout_txmeta,
-		      "event=0x7,instance=0x46,umask=0x4");
-AMDGPU_PMU_ATTR(cake1_pcsout_txmeta,
-		      "event=0x7,instance=0x47,umask=0x4");
-AMDGPU_PMU_ATTR(cake0_ftiinstat_reqalloc,
-		      "event=0xb,instance=0x46,umask=0x4");
-AMDGPU_PMU_ATTR(cake1_ftiinstat_reqalloc,
-		      "event=0xb,instance=0x47,umask=0x4");
-AMDGPU_PMU_ATTR(cake0_ftiinstat_rspalloc,
-		      "event=0xb,instance=0x46,umask=0x8");
-AMDGPU_PMU_ATTR(cake1_ftiinstat_rspalloc,
-		      "event=0xb,instance=0x47,umask=0x8");
-
-/* df event attributes  */
-static struct attribute *df_v3_6_event_attrs[] = {
-	&pmu_attr_cake0_pcsout_txdata.attr,
-	&pmu_attr_cake1_pcsout_txdata.attr,
-	&pmu_attr_cake0_pcsout_txmeta.attr,
-	&pmu_attr_cake1_pcsout_txmeta.attr,
-	&pmu_attr_cake0_ftiinstat_reqalloc.attr,
-	&pmu_attr_cake1_ftiinstat_reqalloc.attr,
-	&pmu_attr_cake0_ftiinstat_rspalloc.attr,
-	&pmu_attr_cake1_ftiinstat_rspalloc.attr,
-	NULL
-};
-
-/* df event attribute group */
-static struct attribute_group df_v3_6_event_attr_group = {
-	.name = "events",
-	.attrs = df_v3_6_event_attrs
-};
+/* get flags from df perfmon config */
+#define DF_V3_6_GET_EVENT(x)		(x & 0xFFUL)
+#define DF_V3_6_GET_INSTANCE(x)		((x >> 8) & 0xFFUL)
+#define DF_V3_6_GET_UNITMASK(x)		((x >> 16) & 0xFFUL)
+#define DF_V3_6_PERFMON_OVERFLOW	0xFFFFFFFFFFFFULL
 
-/* df event attr groups  */
-const struct attribute_group *df_v3_6_attr_groups[] = {
-		&df_v3_6_format_attr_group,
-		&df_v3_6_event_attr_group,
-		NULL
-};
+static u32 df_v3_6_channel_number[] = {1, 2, 0, 4, 0, 8, 0,
+				       16, 32, 0, 0, 0, 2, 4, 8};
 
 static uint64_t df_v3_6_get_fica(struct amdgpu_device *adev,
 				 uint32_t ficaa_val)
diff --git a/drivers/gpu/drm/amd/amdgpu/df_v3_6.h b/drivers/gpu/drm/amd/amdgpu/df_v3_6.h
index 76998541bc30..2505c7ef258a 100644
--- a/drivers/gpu/drm/amd/amdgpu/df_v3_6.h
+++ b/drivers/gpu/drm/amd/amdgpu/df_v3_6.h
@@ -35,15 +35,6 @@ enum DF_V3_6_MGCG {
 	DF_V3_6_MGCG_ENABLE_63_CYCLE_DELAY = 15
 };
 
-/* Defined in global_features.h as FTI_PERFMON_VISIBLE */
-#define DF_V3_6_MAX_COUNTERS		4
-
-/* get flags from df perfmon config */
-#define DF_V3_6_GET_EVENT(x)		(x & 0xFFUL)
-#define DF_V3_6_GET_INSTANCE(x)		((x >> 8) & 0xFFUL)
-#define DF_V3_6_GET_UNITMASK(x)		((x >> 16) & 0xFFUL)
-#define DF_V3_6_PERFMON_OVERFLOW	0xFFFFFFFFFFFFULL
-
 extern const struct attribute_group *df_v3_6_attr_groups[];
 extern const struct amdgpu_df_funcs df_v3_6_funcs;
 
-- 
2.17.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx