diff mbox series

[3/5] arm64: perf: Support new DT compatibles

Message ID 579f301dbf5347d20cfdf49480b850cba82c1ca2.1638900542.git.robin.murphy@arm.com (mailing list archive)
State New, archived
Headers show
Series arm64: DT binding/PMU updates | expand

Commit Message

Robin Murphy Dec. 7, 2021, 6:20 p.m. UTC 
Wire up the new DT compatibles so we can present appropriate
PMU names to userspace for the latest and greatest CPUs.

Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
 arch/arm64/kernel/perf_event.c | 36 ++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

Comments

Mark Rutland Dec. 7, 2021, 6:44 p.m. UTC | #1 
On Tue, Dec 07, 2021 at 06:20:41PM +0000, Robin Murphy wrote:
> Wire up the new DT compatibles so we can present appropriate
> PMU names to userspace for the latest and greatest CPUs.
> 
> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
> ---
>  arch/arm64/kernel/perf_event.c | 36 ++++++++++++++++++++++++++++++++++
>  1 file changed, 36 insertions(+)
> 
> diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c
> index 57720372da62..3fe4dcfc28d4 100644
> --- a/arch/arm64/kernel/perf_event.c
> +++ b/arch/arm64/kernel/perf_event.c
> @@ -1215,6 +1215,26 @@ static int armv8_a78_pmu_init(struct arm_pmu *cpu_pmu)
>  	return armv8_pmu_init_nogroups(cpu_pmu, "armv8_cortex_a78", NULL);
>  }
>  
> +static int armv9_a510_pmu_init(struct arm_pmu *cpu_pmu)
> +{
> +	return armv8_pmu_init_nogroups(cpu_pmu, "armv9_cortex_a510", NULL);
> +}
> +
> +static int armv9_a710_pmu_init(struct arm_pmu *cpu_pmu)
> +{
> +	return armv8_pmu_init_nogroups(cpu_pmu, "armv9_cortex_a710", NULL);
> +}
> +
> +static int armv8_x1_pmu_init(struct arm_pmu *cpu_pmu)
> +{
> +	return armv8_pmu_init_nogroups(cpu_pmu, "armv8_cortex_x1", NULL);
> +}
> +
> +static int armv9_x2_pmu_init(struct arm_pmu *cpu_pmu)
> +{
> +	return armv8_pmu_init_nogroups(cpu_pmu, "armv9_cortex_x2", NULL);
> +}

I wonder if it'd be better to do something like:

#define PMU_INIT_SIMPLE(name)						\
static int name##_pmu_init(struct arm_pmu *cpu_pmu)			\
{
	return armv8_pmu_init_nogroups(cpu_pmu, #name, NULL);		\
}

PMU_INIT_SIMPLE(armv9_cortex_a510)
PMU_INIT_SIMPLE(armv9_cortex_a710)
PMU_INIT_SIMPLE(armv8_xortex_x1)
PMU_INIT_SIMPLE(armv9_xortex_x2)

... and fix up the armv8_pmu_of_device_ids[] table to use the longer init names
that results in?

Otherwise, looks good to me.

Thanks,
Mark.

> +
>  static int armv8_e1_pmu_init(struct arm_pmu *cpu_pmu)
>  {
>  	return armv8_pmu_init_nogroups(cpu_pmu, "armv8_neoverse_e1", NULL);
> @@ -1225,6 +1245,16 @@ static int armv8_n1_pmu_init(struct arm_pmu *cpu_pmu)
>  	return armv8_pmu_init_nogroups(cpu_pmu, "armv8_neoverse_n1", NULL);
>  }
>  
> +static int armv9_n2_pmu_init(struct arm_pmu *cpu_pmu)
> +{
> +	return armv8_pmu_init_nogroups(cpu_pmu, "armv9_neoverse_n2", NULL);
> +}
> +
> +static int armv8_v1_pmu_init(struct arm_pmu *cpu_pmu)
> +{
> +	return armv8_pmu_init_nogroups(cpu_pmu, "armv8_neoverse_v1", NULL);
> +}
> +
>  static int armv8_thunder_pmu_init(struct arm_pmu *cpu_pmu)
>  {
>  	return armv8_pmu_init_nogroups(cpu_pmu, "armv8_cavium_thunder",
> @@ -1251,8 +1281,14 @@ static const struct of_device_id armv8_pmu_of_device_ids[] = {
>  	{.compatible = "arm,cortex-a76-pmu",	.data = armv8_a76_pmu_init},
>  	{.compatible = "arm,cortex-a77-pmu",	.data = armv8_a77_pmu_init},
>  	{.compatible = "arm,cortex-a78-pmu",	.data = armv8_a78_pmu_init},
> +	{.compatible = "arm,cortex-a510-pmu",	.data = armv9_a510_pmu_init},
> +	{.compatible = "arm,cortex-a710-pmu",	.data = armv9_a710_pmu_init},
> +	{.compatible = "arm,cortex-x1-pmu",	.data = armv8_x1_pmu_init},
> +	{.compatible = "arm,cortex-x2-pmu",	.data = armv9_x2_pmu_init},
>  	{.compatible = "arm,neoverse-e1-pmu",	.data = armv8_e1_pmu_init},
>  	{.compatible = "arm,neoverse-n1-pmu",	.data = armv8_n1_pmu_init},
> +	{.compatible = "arm,neoverse-n2-pmu",	.data = armv9_n2_pmu_init},
> +	{.compatible = "arm,neoverse-v1-pmu",	.data = armv8_v1_pmu_init},
>  	{.compatible = "cavium,thunder-pmu",	.data = armv8_thunder_pmu_init},
>  	{.compatible = "brcm,vulcan-pmu",	.data = armv8_vulcan_pmu_init},
>  	{},
> -- 
> 2.28.0.dirty
>
Robin Murphy Dec. 7, 2021, 7:14 p.m. UTC | #2 
On 2021-12-07 18:44, Mark Rutland wrote:
> On Tue, Dec 07, 2021 at 06:20:41PM +0000, Robin Murphy wrote:
>> Wire up the new DT compatibles so we can present appropriate
>> PMU names to userspace for the latest and greatest CPUs.
>>
>> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
>> ---
>>   arch/arm64/kernel/perf_event.c | 36 ++++++++++++++++++++++++++++++++++
>>   1 file changed, 36 insertions(+)
>>
>> diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c
>> index 57720372da62..3fe4dcfc28d4 100644
>> --- a/arch/arm64/kernel/perf_event.c
>> +++ b/arch/arm64/kernel/perf_event.c
>> @@ -1215,6 +1215,26 @@ static int armv8_a78_pmu_init(struct arm_pmu *cpu_pmu)
>>   	return armv8_pmu_init_nogroups(cpu_pmu, "armv8_cortex_a78", NULL);
>>   }
>>   
>> +static int armv9_a510_pmu_init(struct arm_pmu *cpu_pmu)
>> +{
>> +	return armv8_pmu_init_nogroups(cpu_pmu, "armv9_cortex_a510", NULL);
>> +}
>> +
>> +static int armv9_a710_pmu_init(struct arm_pmu *cpu_pmu)
>> +{
>> +	return armv8_pmu_init_nogroups(cpu_pmu, "armv9_cortex_a710", NULL);
>> +}
>> +
>> +static int armv8_x1_pmu_init(struct arm_pmu *cpu_pmu)
>> +{
>> +	return armv8_pmu_init_nogroups(cpu_pmu, "armv8_cortex_x1", NULL);
>> +}
>> +
>> +static int armv9_x2_pmu_init(struct arm_pmu *cpu_pmu)
>> +{
>> +	return armv8_pmu_init_nogroups(cpu_pmu, "armv9_cortex_x2", NULL);
>> +}
> 
> I wonder if it'd be better to do something like:
> 
> #define PMU_INIT_SIMPLE(name)						\
> static int name##_pmu_init(struct arm_pmu *cpu_pmu)			\
> {
> 	return armv8_pmu_init_nogroups(cpu_pmu, #name, NULL);		\
> }
> 
> PMU_INIT_SIMPLE(armv9_cortex_a510)
> PMU_INIT_SIMPLE(armv9_cortex_a710)
> PMU_INIT_SIMPLE(armv8_xortex_x1)
> PMU_INIT_SIMPLE(armv9_xortex_x2)
> 
> ... and fix up the armv8_pmu_of_device_ids[] table to use the longer init names
> that results in?

Indeed I did ponder doing almost exactly that, but at that point I'd 
rather try refactoring a bit deeper to convert most of the arm_pmu init 
business to pure data, so I figured I'd chuck in the simple tweak to 
mitigate these new additions with minimal churn, then have a go at the 
bigger change in its own right.

> Otherwise, looks good to me.

Thanks!

Robin.

> 
> Thanks,
> Mark.
> 
>> +
>>   static int armv8_e1_pmu_init(struct arm_pmu *cpu_pmu)
>>   {
>>   	return armv8_pmu_init_nogroups(cpu_pmu, "armv8_neoverse_e1", NULL);
>> @@ -1225,6 +1245,16 @@ static int armv8_n1_pmu_init(struct arm_pmu *cpu_pmu)
>>   	return armv8_pmu_init_nogroups(cpu_pmu, "armv8_neoverse_n1", NULL);
>>   }
>>   
>> +static int armv9_n2_pmu_init(struct arm_pmu *cpu_pmu)
>> +{
>> +	return armv8_pmu_init_nogroups(cpu_pmu, "armv9_neoverse_n2", NULL);
>> +}
>> +
>> +static int armv8_v1_pmu_init(struct arm_pmu *cpu_pmu)
>> +{
>> +	return armv8_pmu_init_nogroups(cpu_pmu, "armv8_neoverse_v1", NULL);
>> +}
>> +
>>   static int armv8_thunder_pmu_init(struct arm_pmu *cpu_pmu)
>>   {
>>   	return armv8_pmu_init_nogroups(cpu_pmu, "armv8_cavium_thunder",
>> @@ -1251,8 +1281,14 @@ static const struct of_device_id armv8_pmu_of_device_ids[] = {
>>   	{.compatible = "arm,cortex-a76-pmu",	.data = armv8_a76_pmu_init},
>>   	{.compatible = "arm,cortex-a77-pmu",	.data = armv8_a77_pmu_init},
>>   	{.compatible = "arm,cortex-a78-pmu",	.data = armv8_a78_pmu_init},
>> +	{.compatible = "arm,cortex-a510-pmu",	.data = armv9_a510_pmu_init},
>> +	{.compatible = "arm,cortex-a710-pmu",	.data = armv9_a710_pmu_init},
>> +	{.compatible = "arm,cortex-x1-pmu",	.data = armv8_x1_pmu_init},
>> +	{.compatible = "arm,cortex-x2-pmu",	.data = armv9_x2_pmu_init},
>>   	{.compatible = "arm,neoverse-e1-pmu",	.data = armv8_e1_pmu_init},
>>   	{.compatible = "arm,neoverse-n1-pmu",	.data = armv8_n1_pmu_init},
>> +	{.compatible = "arm,neoverse-n2-pmu",	.data = armv9_n2_pmu_init},
>> +	{.compatible = "arm,neoverse-v1-pmu",	.data = armv8_v1_pmu_init},
>>   	{.compatible = "cavium,thunder-pmu",	.data = armv8_thunder_pmu_init},
>>   	{.compatible = "brcm,vulcan-pmu",	.data = armv8_vulcan_pmu_init},
>>   	{},
>> -- 
>> 2.28.0.dirty
>>
Mark Rutland Dec. 14, 2021, 1:36 p.m. UTC | #3 
On Tue, Dec 07, 2021 at 07:14:29PM +0000, Robin Murphy wrote:
> On 2021-12-07 18:44, Mark Rutland wrote:
> > On Tue, Dec 07, 2021 at 06:20:41PM +0000, Robin Murphy wrote:
> > > Wire up the new DT compatibles so we can present appropriate
> > > PMU names to userspace for the latest and greatest CPUs.
> > > 
> > > Signed-off-by: Robin Murphy <robin.murphy@arm.com>
> > > ---
> > >   arch/arm64/kernel/perf_event.c | 36 ++++++++++++++++++++++++++++++++++
> > >   1 file changed, 36 insertions(+)
> > > 
> > > diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c
> > > index 57720372da62..3fe4dcfc28d4 100644
> > > --- a/arch/arm64/kernel/perf_event.c
> > > +++ b/arch/arm64/kernel/perf_event.c
> > > @@ -1215,6 +1215,26 @@ static int armv8_a78_pmu_init(struct arm_pmu *cpu_pmu)
> > >   	return armv8_pmu_init_nogroups(cpu_pmu, "armv8_cortex_a78", NULL);
> > >   }
> > > +static int armv9_a510_pmu_init(struct arm_pmu *cpu_pmu)
> > > +{
> > > +	return armv8_pmu_init_nogroups(cpu_pmu, "armv9_cortex_a510", NULL);
> > > +}
> > > +
> > > +static int armv9_a710_pmu_init(struct arm_pmu *cpu_pmu)
> > > +{
> > > +	return armv8_pmu_init_nogroups(cpu_pmu, "armv9_cortex_a710", NULL);
> > > +}
> > > +
> > > +static int armv8_x1_pmu_init(struct arm_pmu *cpu_pmu)
> > > +{
> > > +	return armv8_pmu_init_nogroups(cpu_pmu, "armv8_cortex_x1", NULL);
> > > +}
> > > +
> > > +static int armv9_x2_pmu_init(struct arm_pmu *cpu_pmu)
> > > +{
> > > +	return armv8_pmu_init_nogroups(cpu_pmu, "armv9_cortex_x2", NULL);
> > > +}
> > 
> > I wonder if it'd be better to do something like:
> > 
> > #define PMU_INIT_SIMPLE(name)						\
> > static int name##_pmu_init(struct arm_pmu *cpu_pmu)			\
> > {
> > 	return armv8_pmu_init_nogroups(cpu_pmu, #name, NULL);		\
> > }
> > 
> > PMU_INIT_SIMPLE(armv9_cortex_a510)
> > PMU_INIT_SIMPLE(armv9_cortex_a710)
> > PMU_INIT_SIMPLE(armv8_xortex_x1)
> > PMU_INIT_SIMPLE(armv9_xortex_x2)
> > 
> > ... and fix up the armv8_pmu_of_device_ids[] table to use the longer init names
> > that results in?
> 
> Indeed I did ponder doing almost exactly that, but at that point I'd rather
> try refactoring a bit deeper to convert most of the arm_pmu init business to
> pure data, so I figured I'd chuck in the simple tweak to mitigate these new
> additions with minimal churn, then have a go at the bigger change in its own
> right.

Sure; that makes sense to me, so for this as-is:

Acked-by: Mark Rutland <mark.rutland@arm.com>

... and I'll leave it to Will to have the final say on whether we want the
"armv9_" prefix or whether we stick with "armv8_" for consistenct, when he
chooses to pick this.

One thing I've just realised is that for the ACPI case, we're stuck with
"armv8_pmuv3_%d" regardless, which I think is fine itself, but we might want to
call that out.

Thanks,
Mark.
diff mbox series

Patch

diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c
index 57720372da62..3fe4dcfc28d4 100644
--- a/arch/arm64/kernel/perf_event.c
+++ b/arch/arm64/kernel/perf_event.c
@@ -1215,6 +1215,26 @@  static int armv8_a78_pmu_init(struct arm_pmu *cpu_pmu)
 	return armv8_pmu_init_nogroups(cpu_pmu, "armv8_cortex_a78", NULL);
 }
 
+static int armv9_a510_pmu_init(struct arm_pmu *cpu_pmu)
+{
+	return armv8_pmu_init_nogroups(cpu_pmu, "armv9_cortex_a510", NULL);
+}
+
+static int armv9_a710_pmu_init(struct arm_pmu *cpu_pmu)
+{
+	return armv8_pmu_init_nogroups(cpu_pmu, "armv9_cortex_a710", NULL);
+}
+
+static int armv8_x1_pmu_init(struct arm_pmu *cpu_pmu)
+{
+	return armv8_pmu_init_nogroups(cpu_pmu, "armv8_cortex_x1", NULL);
+}
+
+static int armv9_x2_pmu_init(struct arm_pmu *cpu_pmu)
+{
+	return armv8_pmu_init_nogroups(cpu_pmu, "armv9_cortex_x2", NULL);
+}
+
 static int armv8_e1_pmu_init(struct arm_pmu *cpu_pmu)
 {
 	return armv8_pmu_init_nogroups(cpu_pmu, "armv8_neoverse_e1", NULL);
@@ -1225,6 +1245,16 @@  static int armv8_n1_pmu_init(struct arm_pmu *cpu_pmu)
 	return armv8_pmu_init_nogroups(cpu_pmu, "armv8_neoverse_n1", NULL);
 }
 
+static int armv9_n2_pmu_init(struct arm_pmu *cpu_pmu)
+{
+	return armv8_pmu_init_nogroups(cpu_pmu, "armv9_neoverse_n2", NULL);
+}
+
+static int armv8_v1_pmu_init(struct arm_pmu *cpu_pmu)
+{
+	return armv8_pmu_init_nogroups(cpu_pmu, "armv8_neoverse_v1", NULL);
+}
+
 static int armv8_thunder_pmu_init(struct arm_pmu *cpu_pmu)
 {
 	return armv8_pmu_init_nogroups(cpu_pmu, "armv8_cavium_thunder",
@@ -1251,8 +1281,14 @@  static const struct of_device_id armv8_pmu_of_device_ids[] = {
 	{.compatible = "arm,cortex-a76-pmu",	.data = armv8_a76_pmu_init},
 	{.compatible = "arm,cortex-a77-pmu",	.data = armv8_a77_pmu_init},
 	{.compatible = "arm,cortex-a78-pmu",	.data = armv8_a78_pmu_init},
+	{.compatible = "arm,cortex-a510-pmu",	.data = armv9_a510_pmu_init},
+	{.compatible = "arm,cortex-a710-pmu",	.data = armv9_a710_pmu_init},
+	{.compatible = "arm,cortex-x1-pmu",	.data = armv8_x1_pmu_init},
+	{.compatible = "arm,cortex-x2-pmu",	.data = armv9_x2_pmu_init},
 	{.compatible = "arm,neoverse-e1-pmu",	.data = armv8_e1_pmu_init},
 	{.compatible = "arm,neoverse-n1-pmu",	.data = armv8_n1_pmu_init},
+	{.compatible = "arm,neoverse-n2-pmu",	.data = armv9_n2_pmu_init},
+	{.compatible = "arm,neoverse-v1-pmu",	.data = armv8_v1_pmu_init},
 	{.compatible = "cavium,thunder-pmu",	.data = armv8_thunder_pmu_init},
 	{.compatible = "brcm,vulcan-pmu",	.data = armv8_vulcan_pmu_init},
 	{},