diff mbox series

[v10,1/8] dt-bindings: perf: fsl-imx-ddr: Add i.MX95 compatible

Message ID 20240415061320.3948707-1-xu.yang_2@nxp.com
State Not Applicable
Headers show
Series [v10,1/8] dt-bindings: perf: fsl-imx-ddr: Add i.MX95 compatible | expand

Checks

Context Check Description
robh/checkpatch success
robh/patch-applied success
robh/dtbs-check warning build log
robh/dt-meta-schema success

Commit Message

Xu Yang April 15, 2024, 6:13 a.m. UTC 
i.MX95 has a DDR pmu. This will add a compatible for it.

Acked-by: Conor Dooley <conor.dooley@microchip.com>
Signed-off-by: Xu Yang <xu.yang_2@nxp.com>

---
Changes in v2:
 - no changes
Changes in v3:
 - let imx95 compatilbe with imx93
Changes in v4:
 - add Acked-by tag
Changes in v5:
 - no changes
Changes in v6:
 - no changes
Changes in v7:
 - no changes
Changes in v8:
 - no changes
Changes in v9:
 - no changes
Changes in v10:
 - no changes
---
 Documentation/devicetree/bindings/perf/fsl-imx-ddr.yaml | 3 +++
 1 file changed, 3 insertions(+)

Comments

Will Deacon April 19, 2024, 3:49 p.m. UTC | #1 
On Mon, Apr 15, 2024 at 02:13:15PM +0800, Xu Yang wrote:
> In current design, the user of perf app needs to input counter ID to count
> events. However, this is not user-friendly since the user needs to lookup
> the map table to find the counter. Instead of letting the user to input
> the counter, let this driver to manage the counters in this patch.
> 
> This will be implemented by:
>  1. allocate counter 0 for cycle event.
>  2. find unused counter from 1-10 for reference events.
>  3. allocate specific counter for counter-specific events.
> 
> In this patch, counter attr will be kept for back-compatible but all the
> value passed down by counter=<n> will be ignored. To mark counter-specific
> events, counter ID will be encoded into perf_pmu_events_attr.id.
> 
> Reviewed-by: Frank Li <Frank.Li@nxp.com>
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> 
> ---
> Changes in v6:
>  - new patch
> Changes in v7:
>  - no changes
> Changes in v8:
>  - add Rb tag
> Changes in v9:
>  - keep 'counter' attr for back-compatible
> Changes in v10:
>  - add some explanation about 'counter' attr in commit message
> ---
>  drivers/perf/fsl_imx9_ddr_perf.c | 168 ++++++++++++++++++-------------
>  1 file changed, 100 insertions(+), 68 deletions(-)

[...]

> @@ -245,8 +249,8 @@ static const struct attribute_group ddr_perf_events_attr_group = {
>  	.attrs = ddr_perf_events_attrs,
>  };
>  
> -PMU_FORMAT_ATTR(event, "config:0-7");
> -PMU_FORMAT_ATTR(counter, "config:8-15");
> +PMU_FORMAT_ATTR(event, "config:0-15");
> +PMU_FORMAT_ATTR(counter, "config:16-23");

Although these mappings are advertised in sysfs, I don't think we can
change them because userspace could be relying on them. I also can't
find any examples of other PMU drivers in the kernel changing these
mappings after being merged, so please keep tthem the same.

If you need to expand the properties to be 16-bit, then you'll need to
split them into 2x8-bit fields.

Will
Frank Li April 19, 2024, 6:38 p.m. UTC | #2 
On Fri, Apr 19, 2024 at 04:49:13PM +0100, Will Deacon wrote:
> On Mon, Apr 15, 2024 at 02:13:15PM +0800, Xu Yang wrote:
> > In current design, the user of perf app needs to input counter ID to count
> > events. However, this is not user-friendly since the user needs to lookup
> > the map table to find the counter. Instead of letting the user to input
> > the counter, let this driver to manage the counters in this patch.
> > 
> > This will be implemented by:
> >  1. allocate counter 0 for cycle event.
> >  2. find unused counter from 1-10 for reference events.
> >  3. allocate specific counter for counter-specific events.
> > 
> > In this patch, counter attr will be kept for back-compatible but all the
> > value passed down by counter=<n> will be ignored. To mark counter-specific
> > events, counter ID will be encoded into perf_pmu_events_attr.id.
> > 
> > Reviewed-by: Frank Li <Frank.Li@nxp.com>
> > Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> > 
> > ---
> > Changes in v6:
> >  - new patch
> > Changes in v7:
> >  - no changes
> > Changes in v8:
> >  - add Rb tag
> > Changes in v9:
> >  - keep 'counter' attr for back-compatible
> > Changes in v10:
> >  - add some explanation about 'counter' attr in commit message
> > ---
> >  drivers/perf/fsl_imx9_ddr_perf.c | 168 ++++++++++++++++++-------------
> >  1 file changed, 100 insertions(+), 68 deletions(-)
> 
> [...]
> 
> > @@ -245,8 +249,8 @@ static const struct attribute_group ddr_perf_events_attr_group = {
> >  	.attrs = ddr_perf_events_attrs,
> >  };
> >  
> > -PMU_FORMAT_ATTR(event, "config:0-7");
> > -PMU_FORMAT_ATTR(counter, "config:8-15");
> > +PMU_FORMAT_ATTR(event, "config:0-15");
> > +PMU_FORMAT_ATTR(counter, "config:16-23");
> 
> Although these mappings are advertised in sysfs, I don't think we can
> change them because userspace could be relying on them. I also can't
> find any examples of other PMU drivers in the kernel changing these
> mappings after being merged, so please keep tthem the same.
> 
> If you need to expand the properties to be 16-bit, then you'll need to
> split them into 2x8-bit fields.

I just see tools/perf/tests/pmu.c: { "krava01", "config:0-1,62-63\n", }
So I supposed "config:" supported below format

PMU_FORMAT_ATTR(event, "config:0-7,16-19");

I just want to confim with you because I have not found other perf driver
using such format yet.

Frank

> 
> Will
Will Deacon April 22, 2024, 4:51 p.m. UTC | #3 
On Fri, Apr 19, 2024 at 02:38:19PM -0400, Frank Li wrote:
> On Fri, Apr 19, 2024 at 04:49:13PM +0100, Will Deacon wrote:
> > On Mon, Apr 15, 2024 at 02:13:15PM +0800, Xu Yang wrote:
> > > In current design, the user of perf app needs to input counter ID to count
> > > events. However, this is not user-friendly since the user needs to lookup
> > > the map table to find the counter. Instead of letting the user to input
> > > the counter, let this driver to manage the counters in this patch.
> > > 
> > > This will be implemented by:
> > >  1. allocate counter 0 for cycle event.
> > >  2. find unused counter from 1-10 for reference events.
> > >  3. allocate specific counter for counter-specific events.
> > > 
> > > In this patch, counter attr will be kept for back-compatible but all the
> > > value passed down by counter=<n> will be ignored. To mark counter-specific
> > > events, counter ID will be encoded into perf_pmu_events_attr.id.
> > > 
> > > Reviewed-by: Frank Li <Frank.Li@nxp.com>
> > > Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> > > 
> > > ---
> > > Changes in v6:
> > >  - new patch
> > > Changes in v7:
> > >  - no changes
> > > Changes in v8:
> > >  - add Rb tag
> > > Changes in v9:
> > >  - keep 'counter' attr for back-compatible
> > > Changes in v10:
> > >  - add some explanation about 'counter' attr in commit message
> > > ---
> > >  drivers/perf/fsl_imx9_ddr_perf.c | 168 ++++++++++++++++++-------------
> > >  1 file changed, 100 insertions(+), 68 deletions(-)
> > 
> > [...]
> > 
> > > @@ -245,8 +249,8 @@ static const struct attribute_group ddr_perf_events_attr_group = {
> > >  	.attrs = ddr_perf_events_attrs,
> > >  };
> > >  
> > > -PMU_FORMAT_ATTR(event, "config:0-7");
> > > -PMU_FORMAT_ATTR(counter, "config:8-15");
> > > +PMU_FORMAT_ATTR(event, "config:0-15");
> > > +PMU_FORMAT_ATTR(counter, "config:16-23");
> > 
> > Although these mappings are advertised in sysfs, I don't think we can
> > change them because userspace could be relying on them. I also can't
> > find any examples of other PMU drivers in the kernel changing these
> > mappings after being merged, so please keep tthem the same.
> > 
> > If you need to expand the properties to be 16-bit, then you'll need to
> > split them into 2x8-bit fields.
> 
> I just see tools/perf/tests/pmu.c: { "krava01", "config:0-1,62-63\n", }
> So I supposed "config:" supported below format
> 
> PMU_FORMAT_ATTR(event, "config:0-7,16-19");
> 
> I just want to confim with you because I have not found other perf driver
> using such format yet.

Oh, wow, that's new syntax for me too! Does the perf tool parse it
properly? (and what happens if an older tool sees the new syntax?)

Will
Xu Yang April 23, 2024, 11:43 a.m. UTC | #4 
Hi Will,

On Mon, Apr 22, 2024 at 05:51:53PM +0100, Will Deacon wrote:
> On Fri, Apr 19, 2024 at 02:38:19PM -0400, Frank Li wrote:
> > On Fri, Apr 19, 2024 at 04:49:13PM +0100, Will Deacon wrote:
> > > On Mon, Apr 15, 2024 at 02:13:15PM +0800, Xu Yang wrote:
> > > > In current design, the user of perf app needs to input counter ID to count
> > > > events. However, this is not user-friendly since the user needs to lookup
> > > > the map table to find the counter. Instead of letting the user to input
> > > > the counter, let this driver to manage the counters in this patch.
> > > > 
> > > > This will be implemented by:
> > > >  1. allocate counter 0 for cycle event.
> > > >  2. find unused counter from 1-10 for reference events.
> > > >  3. allocate specific counter for counter-specific events.
> > > > 
> > > > In this patch, counter attr will be kept for back-compatible but all the
> > > > value passed down by counter=<n> will be ignored. To mark counter-specific
> > > > events, counter ID will be encoded into perf_pmu_events_attr.id.
> > > > 
> > > > Reviewed-by: Frank Li <Frank.Li@nxp.com>
> > > > Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> > > > 
> > > > ---
> > > > Changes in v6:
> > > >  - new patch
> > > > Changes in v7:
> > > >  - no changes
> > > > Changes in v8:
> > > >  - add Rb tag
> > > > Changes in v9:
> > > >  - keep 'counter' attr for back-compatible
> > > > Changes in v10:
> > > >  - add some explanation about 'counter' attr in commit message
> > > > ---
> > > >  drivers/perf/fsl_imx9_ddr_perf.c | 168 ++++++++++++++++++-------------
> > > >  1 file changed, 100 insertions(+), 68 deletions(-)
> > > 
> > > [...]
> > > 
> > > > @@ -245,8 +249,8 @@ static const struct attribute_group ddr_perf_events_attr_group = {
> > > >  	.attrs = ddr_perf_events_attrs,
> > > >  };
> > > >  
> > > > -PMU_FORMAT_ATTR(event, "config:0-7");
> > > > -PMU_FORMAT_ATTR(counter, "config:8-15");
> > > > +PMU_FORMAT_ATTR(event, "config:0-15");
> > > > +PMU_FORMAT_ATTR(counter, "config:16-23");
> > > 
> > > Although these mappings are advertised in sysfs, I don't think we can
> > > change them because userspace could be relying on them. I also can't
> > > find any examples of other PMU drivers in the kernel changing these
> > > mappings after being merged, so please keep tthem the same.
> > > 
> > > If you need to expand the properties to be 16-bit, then you'll need to
> > > split them into 2x8-bit fields.
> > 
> > I just see tools/perf/tests/pmu.c: { "krava01", "config:0-1,62-63\n", }
> > So I supposed "config:" supported below format
> > 
> > PMU_FORMAT_ATTR(event, "config:0-7,16-19");
> > 
> > I just want to confim with you because I have not found other perf driver
> > using such format yet.
> 
> Oh, wow, that's new syntax for me too! Does the perf tool parse it
> properly? (and what happens if an older tool sees the new syntax?)

I've tried the new syntax with different perf version (kernel 6.x, 5.x
and 4.x), all the perf tool can parse it properly. Then I'll use this
syntax for my patchset in next version.

Thanks,
Xu Yang

>
> Will
Xu Yang April 23, 2024, 11:45 a.m. UTC | #5 
Hi Frank,

On Fri, Apr 19, 2024 at 02:38:19PM -0400, Frank Li wrote:
> On Fri, Apr 19, 2024 at 04:49:13PM +0100, Will Deacon wrote:
> > On Mon, Apr 15, 2024 at 02:13:15PM +0800, Xu Yang wrote:
> > > In current design, the user of perf app needs to input counter ID to count
> > > events. However, this is not user-friendly since the user needs to lookup
> > > the map table to find the counter. Instead of letting the user to input
> > > the counter, let this driver to manage the counters in this patch.
> > > 
> > > This will be implemented by:
> > >  1. allocate counter 0 for cycle event.
> > >  2. find unused counter from 1-10 for reference events.
> > >  3. allocate specific counter for counter-specific events.
> > > 
> > > In this patch, counter attr will be kept for back-compatible but all the
> > > value passed down by counter=<n> will be ignored. To mark counter-specific
> > > events, counter ID will be encoded into perf_pmu_events_attr.id.
> > > 
> > > Reviewed-by: Frank Li <Frank.Li@nxp.com>
> > > Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> > > 
> > > ---
> > > Changes in v6:
> > >  - new patch
> > > Changes in v7:
> > >  - no changes
> > > Changes in v8:
> > >  - add Rb tag
> > > Changes in v9:
> > >  - keep 'counter' attr for back-compatible
> > > Changes in v10:
> > >  - add some explanation about 'counter' attr in commit message
> > > ---
> > >  drivers/perf/fsl_imx9_ddr_perf.c | 168 ++++++++++++++++++-------------
> > >  1 file changed, 100 insertions(+), 68 deletions(-)
> > 
> > [...]
> > 
> > > @@ -245,8 +249,8 @@ static const struct attribute_group ddr_perf_events_attr_group = {
> > >  	.attrs = ddr_perf_events_attrs,
> > >  };
> > >  
> > > -PMU_FORMAT_ATTR(event, "config:0-7");
> > > -PMU_FORMAT_ATTR(counter, "config:8-15");
> > > +PMU_FORMAT_ATTR(event, "config:0-15");
> > > +PMU_FORMAT_ATTR(counter, "config:16-23");
> > 
> > Although these mappings are advertised in sysfs, I don't think we can
> > change them because userspace could be relying on them. I also can't
> > find any examples of other PMU drivers in the kernel changing these
> > mappings after being merged, so please keep tthem the same.
> > 
> > If you need to expand the properties to be 16-bit, then you'll need to
> > split them into 2x8-bit fields.
> 
> I just see tools/perf/tests/pmu.c: { "krava01", "config:0-1,62-63\n", }
> So I supposed "config:" supported below format
> 
> PMU_FORMAT_ATTR(event, "config:0-7,16-19");

It works.

Thanks,
Xu Yang

> 
> I just want to confim with you because I have not found other perf driver
> using such format yet.
> 
> Frank
> 
> > 
> > Will
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/perf/fsl-imx-ddr.yaml b/Documentation/devicetree/bindings/perf/fsl-imx-ddr.yaml
index 6c96a4204e5d..37e8b98f2cdc 100644
--- a/Documentation/devicetree/bindings/perf/fsl-imx-ddr.yaml
+++ b/Documentation/devicetree/bindings/perf/fsl-imx-ddr.yaml
@@ -30,6 +30,9 @@  properties:
       - items:
           - const: fsl,imx8dxl-ddr-pmu
           - const: fsl,imx8-ddr-pmu
+      - items:
+          - const: fsl,imx95-ddr-pmu
+          - const: fsl,imx93-ddr-pmu
 
   reg:
     maxItems: 1