[PATCH 2/4] perf/arm-cmn: Add CMN-650 support

[Robin Murphy]

On 2022-04-21 08:25, Ilkka Koskinen wrote:
> 
> I still have a couple tiny comments. Otherwise the patch set looks good 
> to me.
> 
> On Mon, 18 Apr 2022, Robin Murphy wrote:
>> Add the identifiers and events for CMN-650, which slots into its
>> evolutionary position between CMN-600 and the 700-series products.
>> Imagine CMN-600 made bigger, and with most of the rough edges smoothed
>> off, but that then balanced out by some bonkers PMU functionality for
>> the new HN-P enhancement in CMN-650r2.
>>
>> Most of the CXG events are actually common to newer revisions of CMN-600
>> too, so they're arguably a little late; oh well.
>>
>> Signed-off-by: Robin Murphy <robin.murphy at arm.com>
>> ---
>> drivers/perf/arm-cmn.c | 222 ++++++++++++++++++++++++++++++++---------
>> 1 file changed, 176 insertions(+), 46 deletions(-)
>>
>> diff --git a/drivers/perf/arm-cmn.c b/drivers/perf/arm-cmn.c
>> index 9c1d82be7a2f..cce8516d465c 100644
>> --- a/drivers/perf/arm-cmn.c
>> +++ b/drivers/perf/arm-cmn.c
>> @@ -39,7 +39,7 @@
>> #define CMN_CHILD_NODE_ADDR        GENMASK(27, 0)
>> #define CMN_CHILD_NODE_EXTERNAL        BIT(31)
>>
>> -#define CMN_MAX_DIMENSION        8
>> +#define CMN_MAX_DIMENSION        12
>> #define CMN_MAX_XPS            (CMN_MAX_DIMENSION * CMN_MAX_DIMENSION)
>> #define CMN_MAX_DTMS            (CMN_MAX_XPS + (CMN_MAX_DIMENSION - 1) 
>> * 4)
>>
>> @@ -65,7 +65,9 @@
>>
>> /* For most nodes, this is all there is */
>> #define CMN_PMU_EVENT_SEL        0x000
>> -#define CMN_PMU_EVENTn_ID_SHIFT(n)    ((n) * 8)
>> +
>> +/* HN-Ps are weird... */
>> +#define CMN_HNP_PMU_EVENT_SEL        0x008
>>
>> /* DTMs live in the PMU space of XP registers */
>> #define CMN_DTM_WPn(n)            (0x1A0 + (n) * 0x18)
>> @@ -177,9 +179,12 @@
>>
>>
>> enum cmn_model {
>> -    CMN_ANY = -1,
>>     CMN600 = 1,
>> -    CI700 = 2,
>> +    CMN650 = 2,
>> +    CI700 = 8,
>> +    /* ...and then we can use bitmap tricks for commonality */
>> +    CMN_ANY = -1,
>> +    NOT_CMN600 = -2,
> 
> Matter of taste, but I'd probably prefer NOT_CMN600 = ~CMN600

Sure, I had it written that way at one point during development when it 
was a separate macro, so even I'm not entirely certain why it ended up 
like this - I must have been feeling particularly whimsical that day.

>> };
>>
>> /* CMN-600 r0px shouldn't exist in silicon, thankfully */
>> @@ -191,6 +196,11 @@ enum cmn_revision {
>>     CMN600_R2P0,
>>     CMN600_R3P0,
>>     CMN600_R3P1,
>> +    CMN650_R0P0 = 0,
>> +    CMN650_R1P0,
>> +    CMN650_R1P1,
>> +    CMN650_R2P0,
>> +    CMN650_R1P2,
>>     CI700_R0P0 = 0,
>>     CI700_R1P0,
>>     CI700_R2P0,
>> @@ -211,6 +221,7 @@ enum cmn_node_type {
>>     CMN_TYPE_RND = 0xd,
>>     CMN_TYPE_RNSAM = 0xf,
>>     CMN_TYPE_MTSX,
>> +    CMN_TYPE_HNP,
>>     CMN_TYPE_CXRA = 0x100,
>>     CMN_TYPE_CXHA = 0x101,
>>     CMN_TYPE_CXLA = 0x102,
>> @@ -307,9 +318,7 @@ struct arm_cmn_nodeid {
> 
> <snip>
> 
>> @@ -580,20 +592,25 @@ static umode_t 
>> arm_cmn_event_attr_is_visible(struct kobject *kobj,
>>     struct device *dev = kobj_to_dev(kobj);
>>     struct arm_cmn *cmn = to_cmn(dev_get_drvdata(dev));
>>     struct arm_cmn_event_attr *eattr;
>> +    enum cmn_node_type type;
>> +    u16 eventid;
>>
>>     eattr = container_of(attr, typeof(*eattr), attr.attr);
>>
>>     if (!(eattr->model & cmn->model))
>>         return 0;
>>
>> +    type = eattr->type;
>> +    eventid = eattr->eventid;
>> +
>>     /* Watchpoints aren't nodes, so avoid confusion */
>> -    if (eattr->type == CMN_TYPE_WP)
>> +    if (type == CMN_TYPE_WP)
>>         return attr->mode;
>>
>>     /* Hide XP events for unused interfaces/channels */
>> -    if (eattr->type == CMN_TYPE_XP) {
>> -        unsigned int intf = (eattr->eventid >> 2) & 7;
>> -        unsigned int chan = eattr->eventid >> 5;
>> +    if (type == CMN_TYPE_XP) {
>> +        unsigned int intf = (eventid >> 2) & 7;
>> +        unsigned int chan = eventid >> 5;
>>
>>         if ((intf & 4) && !(cmn->ports_used & BIT(intf & 3)))
>>             return 0;
>> @@ -607,12 +624,29 @@ static umode_t 
>> arm_cmn_event_attr_is_visible(struct kobject *kobj,
>>     }
>>
>>     /* Revision-specific differences */
>> -    if (cmn->model == CMN600 && cmn->rev < CMN600_R1P2) {
>> -        if (eattr->type == CMN_TYPE_HNF && eattr->eventid == 0x1b)
>> -            return 0;
>> +    if (cmn->model == CMN600) {
>> +        if (cmn->rev < CMN600_R1P3) {
>> +            if (type == CMN_TYPE_CXRA && eventid > 0x10)
>> +                return 0;
>> +        }
>> +        if (cmn->rev < CMN600_R1P2) {
>> +            if (type == CMN_TYPE_HNF && eventid == 0x1b)
>> +                return 0;
>> +            if (type == CMN_TYPE_CXRA || type == CMN_TYPE_CXHA)
>> +                return 0;
>> +        }
>> +    } else if (cmn->model == CMN650) {
>> +        if (cmn->rev < CMN650_R2P0 || cmn->rev == CMN650_R1P2) {
>> +            if (type == CMN_TYPE_HNF && eventid > 0x22)
>> +                return 0;
>> +            if (type == CMN_TYPE_SBSX && eventid == 0x17)
>> +                return 0;
>> +            if (type == CMN_TYPE_RNI && eventid > 0x10)
>> +                return 0;
>> +        }
> 
> What's the plan with cmn-650 r2p0 event settings? There seem to be a few 
> extra ones made visible now. I'm fine with updating this patch or taking 
> care of them in a separate one, which ever makes more sense.

arm_cmn_event_attrs *should* contain all the events supported by r2p0, 
with the ones not present on earlier revisions then being filtered out 
here. Have I missed anything? Note that HN-Ps don't need per-event 
filtering since that node type simply doesn't exist prior to CMN-650 r2, 
so is already filtered by arm_cmn_node().

Thanks,
Robin.