Re: [PATCH 1/8] perf/arm-cmn: Refactor node ID handling. Again.

From: Ilkka Koskinen
Date: Fri Aug 23 2024 - 21:00:51 EST




On Fri, 9 Aug 2024, Robin Murphy wrote:
It transpires that despite the explicit example to the contrary in the
CMN-700 TRM, the "extra device ports" config is in fact a per-XP thing
rather than a global one. To that end, rework node IDs yet again to
carry around the additional data necessary to decode them properly. At
this point the notion of fully decomposing an ID becomes more
impractical than it's worth, so unabstracting the XY mesh coordinates
(where 2/3 users were just debug anyway) ends up leaving things a bit
simpler overall.

Fixes: 60d1504070c2 ("perf/arm-cmn: Support new IP features")
Signed-off-by: Robin Murphy <robin.murphy@xxxxxxx>
---
drivers/perf/arm-cmn.c | 94 ++++++++++++++++++------------------------
1 file changed, 40 insertions(+), 54 deletions(-)

diff --git a/drivers/perf/arm-cmn.c b/drivers/perf/arm-cmn.c
index c932d9d355cf..fd2122a37f22 100644
--- a/drivers/perf/arm-cmn.c
+++ b/drivers/perf/arm-cmn.c

@@ -357,49 +352,33 @@ struct arm_cmn {
static int arm_cmn_hp_state;

struct arm_cmn_nodeid {
- u8 x;
- u8 y;
u8 port;
u8 dev;
};

static int arm_cmn_xyidbits(const struct arm_cmn *cmn)
{
- return fls((cmn->mesh_x - 1) | (cmn->mesh_y - 1) | 2);
+ return fls((cmn->mesh_x - 1) | (cmn->mesh_y - 1));
}

-static struct arm_cmn_nodeid arm_cmn_nid(const struct arm_cmn *cmn, u16 id)
+static struct arm_cmn_nodeid arm_cmn_nid(const struct arm_cmn_node *dn)
{
struct arm_cmn_nodeid nid;

- if (cmn->num_xps == 1) {
- nid.x = 0;
- nid.y = 0;
- nid.port = CMN_NODEID_1x1_PID(id);
- nid.dev = CMN_NODEID_DEVID(id);
- } else {
- int bits = arm_cmn_xyidbits(cmn);
-
- nid.x = CMN_NODEID_X(id, bits);
- nid.y = CMN_NODEID_Y(id, bits);
- if (cmn->ports_used & 0xc) {
- nid.port = CMN_NODEID_EXT_PID(id);
- nid.dev = CMN_NODEID_EXT_DEVID(id);
- } else {
- nid.port = CMN_NODEID_PID(id);
- nid.dev = CMN_NODEID_DEVID(id);
- }
- }
+ nid.dev = dn->id & ((1U << dn->deviceid_bits) - 1);
+ nid.port = (dn->id >> dn->deviceid_bits) & ((1U << dn->portid_bits) - 1);
return nid;
}

static struct arm_cmn_node *arm_cmn_node_to_xp(const struct arm_cmn *cmn,
const struct arm_cmn_node *dn)
{
- struct arm_cmn_nodeid nid = arm_cmn_nid(cmn, dn->id);
- int xp_idx = cmn->mesh_x * nid.y + nid.x;
+ int id = dn->id >> (dn->portid_bits + dn->deviceid_bits);
+ int bits = arm_cmn_xyidbits(cmn);
+ int x = id > bits;
^^^

Instead of comparison, shouldn't that be bit shift?

Cheers, Ilkka

+ int y = id & ((1U << bits) - 1);

- return cmn->xps + xp_idx;
+ return cmn->xps + cmn->mesh_x * y + x;
}
static struct arm_cmn_node *arm_cmn_node(const struct arm_cmn *cmn,
enum cmn_node_type type)