Received-SPF: pass (google.com: domain of linux-kernel+bounces-86547-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
From: Xu Yang <xu.yang_2@nxp.com>
To: Will Deacon <will@kernel.org>
CC: Frank Li <frank.li@nxp.com>, "mark.rutland@arm.com"
	<mark.rutland@arm.com>, "robh+dt@kernel.org" <robh+dt@kernel.org>,
	"krzysztof.kozlowski+dt@linaro.org" <krzysztof.kozlowski+dt@linaro.org>,
	"conor+dt@kernel.org" <conor+dt@kernel.org>, "shawnguo@kernel.org"
	<shawnguo@kernel.org>, "s.hauer@pengutronix.de" <s.hauer@pengutronix.de>,
	"kernel@pengutronix.de" <kernel@pengutronix.de>, "festevam@gmail.com"
	<festevam@gmail.com>, "john.g.garry@oracle.com" <john.g.garry@oracle.com>,
	"jolsa@kernel.org" <jolsa@kernel.org>, "namhyung@kernel.org"
	<namhyung@kernel.org>, "irogers@google.com" <irogers@google.com>,
	dl-linux-imx <linux-imx@nxp.com>, "mike.leach@linaro.org"
	<mike.leach@linaro.org>, "leo.yan@linaro.org" <leo.yan@linaro.org>,
	"peterz@infradead.org" <peterz@infradead.org>, "mingo@redhat.com"
	<mingo@redhat.com>, "acme@kernel.org" <acme@kernel.org>,
	"alexander.shishkin@linux.intel.com" <alexander.shishkin@linux.intel.com>,
	"adrian.hunter@intel.com" <adrian.hunter@intel.com>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>, "devicetree@vger.kernel.org"
	<devicetree@vger.kernel.org>, "linux-kernel@vger.kernel.org"
	<linux-kernel@vger.kernel.org>, "linux-perf-users@vger.kernel.org"
	<linux-perf-users@vger.kernel.org>, "imx@lists.linux.dev"
	<imx@lists.linux.dev>
Subject: RE: [EXT] Re: [PATCH v4 3/6] perf: imx_perf: add support for i.MX95
 platform
Thread-Topic: [EXT] Re: [PATCH v4 3/6] perf: imx_perf: add support for i.MX95
 platform
Thread-Index: AQHaVAmbPv/9ruMYiUqsh+Jjk7/Y+7EWa6UAgArfnqA=
Date: Thu, 29 Feb 2024 10:27:03 +0000
Message-ID:
 <DU2PR04MB882256B6BA4330F319CF61EB8C5F2@DU2PR04MB8822.eurprd04.prod.outlook.com>
References: <20240131055811.3035741-1-xu.yang_2@nxp.com>
 <20240131055811.3035741-3-xu.yang_2@nxp.com>
 <20240222122253.GB8308@willie-the-truck>
In-Reply-To: <20240222122253.GB8308@willie-the-truck>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0:
 =?us-ascii?Q?rAxZKl5gEIYMigAO5R8iA6oJkQYZQCQ6uRX5hOCKoNaZpxdagGlEbKWE3efx?=
 =?us-ascii?Q?CpP35a88O+gLK72DfCPGroYQyzT2NnpQOjD8hyZIwODaK9HjmB2/ui9K7XH9?=
 =?us-ascii?Q?BpC9vfoEbREprxMM5dPDCBoKVDjuttqHXXOUhUrt6gUsLmSKWipPdHP/rIKh?=
 =?us-ascii?Q?AKpQnJAi4xuFTPZ0XAwEpYbz/7mitm/NPSQs+guly9p13PMfCxBWS7M+DUh2?=
 =?us-ascii?Q?guccJqrYgvVp3HhzWlmLcLdQiXZFpyfrNcIY1TB+SVsI9EDmkc6nswo70ZOQ?=
 =?us-ascii?Q?YFg+66gqAnM0TozKAFHo+4aO8zzOTYJNj74LPaggK1XSiA+P5KgCkut9qLe8?=
 =?us-ascii?Q?HowBwwg5qNYDDyWgpLw51PuUcxX93E3+rf4R69TpwQRhf9dzlJsp2Lc0nVvM?=
 =?us-ascii?Q?G7DyWao9cXPXRBWMGDAovVWIejTD6/alx9+muC3d9/Pu5C4PuEF0mMf96FIf?=
 =?us-ascii?Q?393xsz292KD46SP11OEo6BX6vPPbadktG1JxhU3Yr+G8SZV8O2yxsqCf+EnK?=
 =?us-ascii?Q?6U3ZXzWZ1pWWQXq8p4llwKuzqUbq28rj72aWgztLo1UJ10CdEHEX0oXSKLMK?=
 =?us-ascii?Q?hsgx8W1Vq85vOf6BJKS6fKMapHMRI9OowJTmKAZ5VEuv3RL0s9i9u6D/n8ju?=
 =?us-ascii?Q?I+/gDjnXeMG1bKRsX63SunOhN7xSHKdzTz2cmHR3ieogNRQh5RHWLmAQSvt6?=
 =?us-ascii?Q?1uzSm3ALafrFgUgKB9RZSOZAyHi7KiR4POx6wf/CRDEE2BCJYz8z5nxsOPOX?=
 =?us-ascii?Q?2kars4z1p8hvoqIwe4DsBb/r2BkO0xHZripm5vkoT/JX4JRD/incwIY6SVCX?=
 =?us-ascii?Q?iYXeqGCrRpjITvRvuT2xvf+aZ8T3RuwIaq6qSkatAi0EVmcgCi+wR7b5OJ+m?=
 =?us-ascii?Q?Ltfi/6hhMz/+ro/4gmbIyLdIND9zOlUKlo0E+QMdgsM5AP9uRS2jS1qkOWNt?=
 =?us-ascii?Q?B09z4KHd6WGCjHE9TLZPWvFPRIXRKgwhZswLDKg/Mrfn2huqKzTcbX4x6vj/?=
 =?us-ascii?Q?NldXVPkU9lIcJkkyIQSaNon9aHTj3pOMombjfsV9zjv8k2UfPekXTnt40AtD?=
 =?us-ascii?Q?WbYnwGp/0dOsrxWHEZHI5IyTS70NS+qPd0+QXU4zNID7OSXH+nADEldzWRvN?=
 =?us-ascii?Q?WcTfGrm8TckycuYuTdURTxqiE0KwBg9WshpfOgY1vgXLi4D1Lq+y6FknYXDQ?=
 =?us-ascii?Q?J+utVTYUUw5380NSpFbJm71vQg8VIq9FUtC8RCEmSk0Dv5CnNFBUXvG5mM0Y?=
 =?us-ascii?Q?gf6uFFd2LWEwllFvCErX4NB7DOJVxEXPxMew/x99LwSCF3hzxpJmR8EEMb8E?=
 =?us-ascii?Q?jSG8aEuBcRujQVN2a9pE0TP+nu989vUP3eHgPeabHVg7dE+euxXUCl8evw7S?=
 =?us-ascii?Q?Q+huEQv6k5ImYAh0BnR6PMeRLlT69V4un71xO0W+eunNcoZENB/DF7ngy5lq?=
 =?us-ascii?Q?WaBe9PWVbV8U15z2LiHSKtB2RJVv+NKJqIgbxgZkgxRCQw65HJievaayx7zf?=
 =?us-ascii?Q?4sUpOhTL2ZyRHx7Wxu1kFA/FpayFdZAur+qefaPmFMsIUHUL/RLDKtSLIHyd?=
 =?us-ascii?Q?sB40W7MR2CT1SZSfotA=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk
X-Mailing-List: linux-kernel@vger.kernel.org
List-Id: <linux-kernel.vger.kernel.org>
List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org>
List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org>
MIME-Version: 1.0
X-OriginatorOrg: nxp.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: DU2PR04MB8822.eurprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: ffa584d5-2f73-42ad-932f-08dc3910f8fd
X-MS-Exchange-CrossTenant-originalarrivaltime: 29 Feb 2024 10:27:03.9118
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: YJ9Wxs0Jj30wpTnh1wu6VS2BSiy3CTRi8057612Y4gLnlOm4xT6QodoHJP/U7y2tiaOJc4tGN6k4A0C/ysiNIg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: AS8PR04MB8360

Hi Will,

>=20
> On Wed, Jan 31, 2024 at 01:58:08PM +0800, Xu Yang wrote:
> > i.MX95 has a DDR PMU which is almostly same as i.MX93, it now supports
> > read beat and write beat filter capabilities. This will add support for
> > i.MX95 and enhance the driver to support specific filter handling for i=
t.
> >
> > Usage:
> >
> > For read beat:
> > ~# perf stat -a -I 1000 -e imx9_ddr0/eddrtq_pm_rd_beat_filt2,counter=3D=
3,axi_mask=3DID_MASK,axi_id=3DID/
> > ~# perf stat -a -I 1000 -e imx9_ddr0/eddrtq_pm_rd_beat_filt1,counter=3D=
4,axi_mask=3DID_MASK,axi_id=3DID/
> > ~# perf stat -a -I 1000 -e imx9_ddr0/eddrtq_pm_rd_beat_filt0,counter=3D=
5,axi_mask=3DID_MASK,axi_id=3DID/
> > eg: For edma2: perf stat -a -I 1000 -e imx9_ddr0/eddrtq_pm_rd_beat_filt=
0,counter=3D5,axi_mask=3D0x00f,axi_id=3D0x00c/
> >
> > For write beat:
> > ~# perf stat -a -I 1000 -e imx9_ddr0/eddrtq_pm_wr_beat_filt,counter=3D2=
,axi_mask=3DID_MASK,axi_id=3DID/
> > eg: For edma2: perf stat -a -I 1000 -e imx9_ddr0/eddrtq_pm_wr_beat_filt=
,counter=3D2,axi_mask=3D0x00f,axi_id=3D0x00c/
> >
> > Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> >
> > ---
> > Changes in v2:
> >  - put soc spefific axi filter events to drvdata according
> >    to franks suggestions.
> >  - adjust pmcfg axi_id and axi_mask config
> > Changes in v3:
> >  - no changes
> > Changes in v4:
> >  - only contain imx95 parts
> > ---
> >  drivers/perf/fsl_imx9_ddr_perf.c | 86 +++++++++++++++++++++++++++++++-
> >  1 file changed, 84 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/perf/fsl_imx9_ddr_perf.c b/drivers/perf/fsl_imx9_d=
dr_perf.c
> > index b1a58e9e1617..85aaaef7212f 100644
> > --- a/drivers/perf/fsl_imx9_ddr_perf.c
> > +++ b/drivers/perf/fsl_imx9_ddr_perf.c
> > @@ -17,9 +17,19 @@
> >  #define MX93_PMCFG1_RD_BT_FILT_EN    BIT(29)
> >  #define MX93_PMCFG1_ID_MASK          GENMASK(17, 0)
> >
> > +#define MX95_PMCFG1_WR_BEAT_FILT_EN  BIT(31)
> > +#define MX95_PMCFG1_RD_BEAT_FILT_EN  BIT(30)
> > +
> >  #define PMCFG2                               0x04
> >  #define MX93_PMCFG2_ID                       GENMASK(17, 0)
> >
> > +#define PMCFG3                               0x08
> > +#define PMCFG4                               0x0C
> > +#define PMCFG5                               0x10
> > +#define PMCFG6                               0x14
> > +#define MX95_PMCFG_ID_MASK           GENMASK(9, 0)
> > +#define MX95_PMCFG_ID                        GENMASK(25, 16)
> > +
> >  /* Global control register affects all counters and takes priority ove=
r local control registers */
> >  #define PMGC0                0x40
> >  /* Global control register bits */
> > @@ -240,6 +250,18 @@ static struct attribute *imx93_ddr_perf_events_att=
rs[] =3D {
> >       NULL,
> >  };
> >
> > +static struct attribute *imx95_ddr_perf_events_attrs[] =3D {
> > +     /* counter2 specific events */
> > +     IMX9_DDR_PMU_EVENT_ATTR(eddrtq_pm_wr_beat_filt, 73),
> > +     /* counter3 specific events */
> > +     IMX9_DDR_PMU_EVENT_ATTR(eddrtq_pm_rd_beat_filt2, 73),
> > +     /* counter4 specific events */
> > +     IMX9_DDR_PMU_EVENT_ATTR(eddrtq_pm_rd_beat_filt1, 73),
> > +     /* counter5 specific events */
> > +     IMX9_DDR_PMU_EVENT_ATTR(eddrtq_pm_rd_beat_filt0, 73),
> > +     NULL,
> > +};
> > +
> >  PMU_FORMAT_ATTR(event, "config:0-7");
> >  PMU_FORMAT_ATTR(counter, "config:8-15");
> >  PMU_FORMAT_ATTR(axi_id, "config1:0-17");
> > @@ -271,8 +293,14 @@ static const struct imx_ddr_devtype_data imx93_dev=
type_data =3D {
> >       .attrs =3D imx93_ddr_perf_events_attrs,
> >  };
> >
> > +static const struct imx_ddr_devtype_data imx95_devtype_data =3D {
> > +     .identifier =3D "imx95",
> > +     .attrs =3D imx95_ddr_perf_events_attrs,
> > +};
> > +
> >  static const struct of_device_id imx_ddr_pmu_dt_ids[] =3D {
> >       { .compatible =3D "fsl,imx93-ddr-pmu", .data =3D &imx93_devtype_d=
ata },
> > +     { .compatible =3D "fsl,imx95-ddr-pmu", .data =3D &imx95_devtype_d=
ata },
> >       { /* sentinel */ }
> >  };
> >  MODULE_DEVICE_TABLE(of, imx_ddr_pmu_dt_ids);
> > @@ -410,6 +438,56 @@ static void imx93_ddr_perf_monitor_config(struct d=
dr_pmu *pmu, int cfg, int cfg1
> >       writel(pmcfg2, pmu->base + PMCFG2);
> >  }
> >
> > +static void imx95_ddr_perf_monitor_config(struct ddr_pmu *pmu, int cfg=
, int cfg1, int cfg2)
> > +{
> > +     u32 pmcfg1, pmcfg, offset =3D 0;
> > +     int event, counter;
> > +
> > +     event =3D cfg & 0x000000FF;
> > +     counter =3D (cfg & 0x0000FF00) >> 8;
> > +
> > +     pmcfg1 =3D readl_relaxed(pmu->base + PMCFG1);
> > +
> > +     if (counter =3D=3D 2 && event =3D=3D 73) {
> > +             pmcfg1 |=3D MX95_PMCFG1_WR_BEAT_FILT_EN;
> > +             offset =3D PMCFG3;
> > +     } else if (counter =3D=3D 2 && event !=3D 73) {
> > +             pmcfg1 &=3D ~MX95_PMCFG1_WR_BEAT_FILT_EN;
> > +     }
> > +
> > +     if (counter =3D=3D 3 && event =3D=3D 73) {
> > +             pmcfg1 |=3D MX95_PMCFG1_RD_BEAT_FILT_EN;
> > +             offset =3D PMCFG4;
> > +     } else if (counter =3D=3D 3 && event !=3D 73) {
> > +             pmcfg1 &=3D ~MX95_PMCFG1_RD_BEAT_FILT_EN;
> > +     }
> > +
> > +     if (counter =3D=3D 4 && event =3D=3D 73) {
> > +             pmcfg1 |=3D MX95_PMCFG1_RD_BEAT_FILT_EN;
> > +             offset =3D PMCFG5;
> > +     } else if (counter =3D=3D 4 && event !=3D 73) {
> > +             pmcfg1 &=3D ~MX95_PMCFG1_RD_BEAT_FILT_EN;
> > +     }
> > +
> > +     if (counter =3D=3D 5 && event =3D=3D 73) {
> > +             pmcfg1 |=3D MX95_PMCFG1_RD_BEAT_FILT_EN;
> > +             offset =3D PMCFG6;
> > +     } else if (counter =3D=3D 5 && event !=3D 73) {
> > +             pmcfg1 &=3D ~MX95_PMCFG1_RD_BEAT_FILT_EN;
> > +     }
>=20
> I think this would be much easier to read if you rewrote it as:
>=20
>         switch (counter) {
>         case 2:
>                 ...
>                 break;
>         case 3:
>=20
>=20
> etc.

Okay, I will improve this.

>=20
> > +     writel(pmcfg1, pmu->base + PMCFG1);
>=20
> writel_relaxed()? (since you're already using readl_relaxed()).

Okay.

>=20
> > +
> > +     if (offset) {
> > +             pmcfg =3D readl_relaxed(pmu->base + offset);
> > +             pmcfg &=3D ~(FIELD_PREP(MX95_PMCFG_ID_MASK, 0x3FF) |
> > +                        FIELD_PREP(MX95_PMCFG_ID, 0x3FF));
> > +             pmcfg |=3D (FIELD_PREP(MX95_PMCFG_ID_MASK, cfg2) |
> > +                       FIELD_PREP(MX95_PMCFG_ID, cfg1));
> > +             writel(pmcfg, pmu->base + offset);
>=20
> Similarly.

Okay.

Thanks,
Xu Yang