Messages in this thread | | | Date | Fri, 2 Jun 2023 00:13:36 -0700 (PDT) | From | Ilkka Koskinen <> | Subject | Re: [PATCH v2 5/5] perf: arm_cspmu: ampere_cspmu: Add support for Ampere SoC PMU |
| |
Hi Robin,
On Thu, 1 Jun 2023, Robin Murphy wrote: > On 2023-06-01 04:01, Ilkka Koskinen wrote: > [...] >> +static bool ampere_cspmu_validate_event(struct arm_cspmu *cspmu, >> + struct perf_event *new) >> +{ >> + struct perf_event *curr; >> + unsigned int idx; >> + u32 threshold = 0, rank = 0, bank = 0; >> + >> + /* We compare the global filter settings to existing events */ >> + idx = find_first_bit(cspmu->hw_events.used_ctrs, >> + cspmu->cycle_counter_logical_idx); >> + >> + /* This is the first event */ >> + if (idx == cspmu->cycle_counter_logical_idx) >> + return true; >> + >> + curr = cspmu->hw_events.events[idx]; >> + >> + if (get_filter_enable(new)) { >> + threshold = get_threshold(new); >> + rank = get_rank(new); >> + bank = get_bank(new); >> + } >> + >> + if (get_filter_enable(new) != get_filter_enable(curr) || > > Is there any useful purpose in allowing the user to specify nonzero rank, > bank or threshold values with filter_enable=0? Assuming not, then between > this and ampere_cspmu_set_ev_filter() it appears that you don't need > filter_enable at all.
Not really. I dropped filter_enable at one point but restored it later to match the smmuv3 pmu driver. I totally agree, it's unnecessary and it's better to remove it completely.
Cheers, Ilkka
> > Thanks, > Robin. > >> + get_threshold(curr) != threshold || >> + get_rank(curr) != rank || >> + get_bank(curr) != bank) >> + return false; >> + >> + return true; >> +} >
| |