Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <cover.1689748843.git.sandipan.das@amd.com> <a70b1ef6ab427b768f21401af280d235dd095412.1689748843.git.sandipan.das@amd.com>
 <CAP-5=fX1NQzus3MQapmBrdgEhDG4+XmawgytfugHHOKD0E-COg@mail.gmail.com> <fef0492b-60c3-70ea-c7f1-370bf2734fc3@amd.com>
In-Reply-To: <fef0492b-60c3-70ea-c7f1-370bf2734fc3@amd.com>
From:   Ian Rogers <irogers@google.com>
Date:   Thu, 20 Jul 2023 08:50:54 -0700
Message-ID: <CAP-5=fX=1t51+a9=Fj-xt4zyLw9OG44bu9wRdPsu2hPoh8cNZQ@mail.gmail.com>
Subject: Re: [PATCH 6/6] perf vendor events amd: Add Zen 4 memory controller events
To:     Sandipan Das <sandipan.das@amd.com>
Cc:     linux-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org,
        x86@kernel.org, peterz@infradead.org, mingo@redhat.com,
        acme@kernel.org, mark.rutland@arm.com,
        alexander.shishkin@linux.intel.com, jolsa@kernel.org,
        namhyung@kernel.org, adrian.hunter@intel.com, tglx@linutronix.de,
        bp@alien8.de, dave.hansen@linux.intel.com, hpa@zytor.com,
        eranian@google.com, ananth.narayan@amd.com, ravi.bangoria@amd.com,
        santosh.shukla@amd.com
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Wed, Jul 19, 2023 at 10:23=E2=80=AFPM Sandipan Das <sandipan.das@amd.com=
> wrote:
>
> On 7/19/2023 9:42 PM, Ian Rogers wrote:
> > On Tue, Jul 18, 2023 at 11:58=E2=80=AFPM Sandipan Das <sandipan.das@amd=
.com> wrote:
> >>
> >> Make the jevents parser aware of the Unified Memory Controller (UMC) P=
MU
> >> and add events taken from Section 8.2.1 "UMC Performance Monitor Event=
s"
> >> of the Processor Programming Reference (PPR) for AMD Family 19h Model =
11h
> >> processors. The events capture UMC command activity such as CAS, ACTIV=
ATE,
> >> PRECHARGE etc. while the metrics derive data bus utilization and memor=
y
> >> bandwidth out of these events.
> >>
> >> Signed-off-by: Sandipan Das <sandipan.das@amd.com>
> >
> > Acked-by: Ian Rogers <irogers@google.com>
> >
> >> ---
> >>  .../arch/x86/amdzen4/memory-controller.json   | 101 +++++++++++++++++=
+
> >>  .../arch/x86/amdzen4/recommended.json         |  84 +++++++++++++++
> >>  tools/perf/pmu-events/jevents.py              |   2 +
> >>  3 files changed, 187 insertions(+)
> >>  create mode 100644 tools/perf/pmu-events/arch/x86/amdzen4/memory-cont=
roller.json
> >>
> >> diff --git a/tools/perf/pmu-events/arch/x86/amdzen4/memory-controller.=
json b/tools/perf/pmu-events/arch/x86/amdzen4/memory-controller.json
> >> new file mode 100644
> >> index 000000000000..55263e5e4f69
> >> --- /dev/null
> >> +++ b/tools/perf/pmu-events/arch/x86/amdzen4/memory-controller.json
> >> @@ -0,0 +1,101 @@
> >> +[
> >> +  {
> >> +    "EventName": "umc_mem_clk",
> >> +    "PublicDescription": "Number of memory clock cycles.",
> >> +    "EventCode": "0x00",
> >> +    "PerPkg": "1",
> >> +    "Unit": "UMCPMC"
> >
> > nit: Why use UMCPMC and then rewrite to amd_umc, why not just use "amd_=
umc" ?
> >
>
> I followed the convention that has been historically used for AMD uncore =
PMUs e.g.
> the "Unit" for amd_df is "DFPMC" and for amd_l3 is "L3PMC". I do agree th=
at its
> simpler to use the same naming so will change this. If you prefer, I can =
send out
> a separate patch to change the unit naming for amd_df and amd_l3.

Thanks for the explanation. I don't mind but it is nicer to have fewer
renames imo. If we get rid of one, perhaps we can get rid of them all?
Perhaps merge this and follow up with simplification.

Thanks,
Ian

> - Sandipan
>