Add Support for HiSilicon CPA PMU driver. Json file in perf tool
is applied in
https://lore.kernel.org/linux-arm-kernel/[email protected]/.
Change since v2:
- Associate CPA PMUs with CPUs online, rather than CPUs in the nearest SCCL.
- Move CPA power management to probe and remove function.
- Address the comments from John.
- Link: https://lore.kernel.org/linux-arm-kernel/[email protected]/
Change since v1:
- Address the comments from John.
- Link: https://lore.kernel.org/linux-arm-kernel/[email protected]/
Qi Liu (2):
drivers/perf: hisi: Associate PMUs in SICL with CPUs online
drivers/perf: hisi: Add Support for CPA PMU
drivers/perf/hisilicon/Makefile | 2 +-
drivers/perf/hisilicon/hisi_uncore_cpa_pmu.c | 400 +++++++++++++++++++
drivers/perf/hisilicon/hisi_uncore_pa_pmu.c | 18 +-
drivers/perf/hisilicon/hisi_uncore_pmu.c | 4 +
drivers/perf/hisilicon/hisi_uncore_pmu.h | 1 +
include/linux/cpuhotplug.h | 1 +
6 files changed, 414 insertions(+), 12 deletions(-)
create mode 100644 drivers/perf/hisilicon/hisi_uncore_cpa_pmu.c
--
2.24.0
If a PMU is in SICL, we associate it with all CPUs online, rather
than CPUs in the nearest SCCL.
Signed-off-by: Qi Liu <[email protected]>
---
drivers/perf/hisilicon/hisi_uncore_pa_pmu.c | 18 +++++++-----------
drivers/perf/hisilicon/hisi_uncore_pmu.c | 4 ++++
drivers/perf/hisilicon/hisi_uncore_pmu.h | 1 +
3 files changed, 12 insertions(+), 11 deletions(-)
diff --git a/drivers/perf/hisilicon/hisi_uncore_pa_pmu.c b/drivers/perf/hisilicon/hisi_uncore_pa_pmu.c
index bad99d149172..54c604c0d404 100644
--- a/drivers/perf/hisilicon/hisi_uncore_pa_pmu.c
+++ b/drivers/perf/hisilicon/hisi_uncore_pa_pmu.c
@@ -258,13 +258,12 @@ static int hisi_pa_pmu_init_data(struct platform_device *pdev,
struct hisi_pmu *pa_pmu)
{
/*
- * Use the SCCL_ID and the index ID to identify the PA PMU,
- * while SCCL_ID is the nearst SCCL_ID from this SICL and
- * CPU core is chosen from this SCCL to manage this PMU.
+ * As PA PMU is in SICL, use the SICL_ID and the index ID
+ * to identify the PA PMU.
*/
if (device_property_read_u32(&pdev->dev, "hisilicon,scl-id",
- &pa_pmu->sccl_id)) {
- dev_err(&pdev->dev, "Cannot read sccl-id!\n");
+ &pa_pmu->sicl_id)) {
+ dev_err(&pdev->dev, "Cannot read sicl-id!\n");
return -EINVAL;
}
@@ -275,6 +274,7 @@ static int hisi_pa_pmu_init_data(struct platform_device *pdev,
}
pa_pmu->ccl_id = -1;
+ pa_pmu->sccl_id = -1;
pa_pmu->base = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(pa_pmu->base)) {
@@ -399,13 +399,9 @@ static int hisi_pa_pmu_probe(struct platform_device *pdev)
ret = hisi_pa_pmu_dev_probe(pdev, pa_pmu);
if (ret)
return ret;
- /*
- * PA is attached in SICL and the CPU core is chosen to manage this
- * PMU which is the nearest SCCL, while its SCCL_ID is greater than
- * one with the SICL_ID.
- */
+
name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "hisi_sicl%u_pa%u",
- pa_pmu->sccl_id - 1, pa_pmu->index_id);
+ pa_pmu->sicl_id, pa_pmu->index_id);
if (!name)
return -ENOMEM;
diff --git a/drivers/perf/hisilicon/hisi_uncore_pmu.c b/drivers/perf/hisilicon/hisi_uncore_pmu.c
index a738aeab5c04..31930166c34b 100644
--- a/drivers/perf/hisilicon/hisi_uncore_pmu.c
+++ b/drivers/perf/hisilicon/hisi_uncore_pmu.c
@@ -458,6 +458,10 @@ static bool hisi_pmu_cpu_is_associated_pmu(struct hisi_pmu *hisi_pmu)
{
int sccl_id, ccl_id;
+ /* If SCCL_ID is -1, the PMU is in SICL and can be associated this PMU with any CPU */
+ if (hisi_pmu->sccl_id == -1)
+ return true;
+
if (hisi_pmu->ccl_id == -1) {
/* If CCL_ID is -1, the PMU only shares the same SCCL */
hisi_read_sccl_and_ccl_id(&sccl_id, NULL);
diff --git a/drivers/perf/hisilicon/hisi_uncore_pmu.h b/drivers/perf/hisilicon/hisi_uncore_pmu.h
index 7f5841d6f592..96eeddad55ff 100644
--- a/drivers/perf/hisilicon/hisi_uncore_pmu.h
+++ b/drivers/perf/hisilicon/hisi_uncore_pmu.h
@@ -81,6 +81,7 @@ struct hisi_pmu {
struct device *dev;
struct hlist_node node;
int sccl_id;
+ int sicl_id;
int ccl_id;
void __iomem *base;
/* the ID of the PMU modules */
--
2.24.0
On 21/03/2022 07:01, Qi Liu wrote:
> If a PMU is in SICL, we associate it with all CPUs online, rather
/s/in SICL/in a SICL/ [and all other places, including the code change],
or maybe it is better mention "IO super cluster" as well for people who
would not know or remember.
> than CPUs in the nearest SCCL.
You are not really saying why. I would mention that we do it as it is
not appropiate to associate with a CPU die, and you can also mention
problems that associating with a SICL creates.
Please also mention how changing FW definition is ok at this stage.
>
> Signed-off-by: Qi Liu <[email protected]>
> ---
> drivers/perf/hisilicon/hisi_uncore_pa_pmu.c | 18 +++++++-----------
> drivers/perf/hisilicon/hisi_uncore_pmu.c | 4 ++++
> drivers/perf/hisilicon/hisi_uncore_pmu.h | 1 +
> 3 files changed, 12 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/perf/hisilicon/hisi_uncore_pa_pmu.c b/drivers/perf/hisilicon/hisi_uncore_pa_pmu.c
> index bad99d149172..54c604c0d404 100644
> --- a/drivers/perf/hisilicon/hisi_uncore_pa_pmu.c
> +++ b/drivers/perf/hisilicon/hisi_uncore_pa_pmu.c
> @@ -258,13 +258,12 @@ static int hisi_pa_pmu_init_data(struct platform_device *pdev,
> struct hisi_pmu *pa_pmu)
> {
> /*
> - * Use the SCCL_ID and the index ID to identify the PA PMU,
> - * while SCCL_ID is the nearst SCCL_ID from this SICL and
> - * CPU core is chosen from this SCCL to manage this PMU.
> + * As PA PMU is in SICL, use the SICL_ID and the index ID
"a SICL"
> + * to identify the PA PMU.
> */
> if (device_property_read_u32(&pdev->dev, "hisilicon,scl-id",
> - &pa_pmu->sccl_id)) {
> - dev_err(&pdev->dev, "Cannot read sccl-id!\n");
> + &pa_pmu->sicl_id)) {
> + dev_err(&pdev->dev, "Cannot read sicl-id!\n");
> return -EINVAL;
> }
>
> @@ -275,6 +274,7 @@ static int hisi_pa_pmu_init_data(struct platform_device *pdev,
> }
>
> pa_pmu->ccl_id = -1;
> + pa_pmu->sccl_id = -1;
>
> pa_pmu->base = devm_platform_ioremap_resource(pdev, 0);
> if (IS_ERR(pa_pmu->base)) {
> @@ -399,13 +399,9 @@ static int hisi_pa_pmu_probe(struct platform_device *pdev)
> ret = hisi_pa_pmu_dev_probe(pdev, pa_pmu);
> if (ret)
> return ret;
> - /*
> - * PA is attached in SICL and the CPU core is chosen to manage this
> - * PMU which is the nearest SCCL, while its SCCL_ID is greater than
> - * one with the SICL_ID.
> - */
> +
> name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "hisi_sicl%u_pa%u",
> - pa_pmu->sccl_id - 1, pa_pmu->index_id);
> + pa_pmu->sicl_id, pa_pmu->index_id);
> if (!name)
> return -ENOMEM;
>
> diff --git a/drivers/perf/hisilicon/hisi_uncore_pmu.c b/drivers/perf/hisilicon/hisi_uncore_pmu.c
> index a738aeab5c04..31930166c34b 100644
> --- a/drivers/perf/hisilicon/hisi_uncore_pmu.c
> +++ b/drivers/perf/hisilicon/hisi_uncore_pmu.c
> @@ -458,6 +458,10 @@ static bool hisi_pmu_cpu_is_associated_pmu(struct hisi_pmu *hisi_pmu)
> {
> int sccl_id, ccl_id;
>
> + /* If SCCL_ID is -1, the PMU is in SICL and can be associated this PMU with any CPU */
too long, just have "The PMU is in a SICL and has no CPU affinity"
> + if (hisi_pmu->sccl_id == -1)
> + return true;
> +
> if (hisi_pmu->ccl_id == -1) {
> /* If CCL_ID is -1, the PMU only shares the same SCCL */
> hisi_read_sccl_and_ccl_id(&sccl_id, NULL);
> diff --git a/drivers/perf/hisilicon/hisi_uncore_pmu.h b/drivers/perf/hisilicon/hisi_uncore_pmu.h
> index 7f5841d6f592..96eeddad55ff 100644
> --- a/drivers/perf/hisilicon/hisi_uncore_pmu.h
> +++ b/drivers/perf/hisilicon/hisi_uncore_pmu.h
> @@ -81,6 +81,7 @@ struct hisi_pmu {
> struct device *dev;
> struct hlist_node node;
> int sccl_id;
> + int sicl_id;
> int ccl_id;
> void __iomem *base;
> /* the ID of the PMU modules */
Hi John,
Thanks for the comments. some replies inline.
On 2022/4/5 16:18, John Garry wrote:
> On 21/03/2022 07:01, Qi Liu wrote:
>> If a PMU is in SICL, we associate it with all CPUs online, rather
>
> /s/in SICL/in a SICL/ [and all other places, including the code change],
> or maybe it is better mention "IO super cluster" as well for people who
> would not know or remember.
>
got it, will change these description, thanks.
>> than CPUs in the nearest SCCL.
>
> You are not really saying why. I would mention that we do it as it is
> not appropiate to associate with a CPU die, and you can also mention
> problems that associating with a SICL creates.
>
> Please also mention how changing FW definition is ok at this stage.
ok, I'll metion this next time.
>
>>
>> Signed-off-by: Qi Liu <[email protected]>
>> ---
>> drivers/perf/hisilicon/hisi_uncore_pa_pmu.c | 18 +++++++-----------
>> drivers/perf/hisilicon/hisi_uncore_pmu.c | 4 ++++
>> drivers/perf/hisilicon/hisi_uncore_pmu.h | 1 +
>> 3 files changed, 12 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/perf/hisilicon/hisi_uncore_pa_pmu.c
>> b/drivers/perf/hisilicon/hisi_uncore_pa_pmu.c
>> index bad99d149172..54c604c0d404 100644
>> --- a/drivers/perf/hisilicon/hisi_uncore_pa_pmu.c
>> +++ b/drivers/perf/hisilicon/hisi_uncore_pa_pmu.c
>> @@ -258,13 +258,12 @@ static int hisi_pa_pmu_init_data(struct
>> platform_device *pdev,
>> struct hisi_pmu *pa_pmu)
>> {
>> /*
>> - * Use the SCCL_ID and the index ID to identify the PA PMU,
>> - * while SCCL_ID is the nearst SCCL_ID from this SICL and
>> - * CPU core is chosen from this SCCL to manage this PMU.
>> + * As PA PMU is in SICL, use the SICL_ID and the index ID
>
> "a SICL"
got it.
>
>> + * to identify the PA PMU.
>> */
>> if (device_property_read_u32(&pdev->dev, "hisilicon,scl-id",
>> - &pa_pmu->sccl_id)) {
>> - dev_err(&pdev->dev, "Cannot read sccl-id!\n");
>> + &pa_pmu->sicl_id)) {
>> + dev_err(&pdev->dev, "Cannot read sicl-id!\n");
>> return -EINVAL;
>> }
>> @@ -275,6 +274,7 @@ static int hisi_pa_pmu_init_data(struct
>> platform_device *pdev,
>> }
>> pa_pmu->ccl_id = -1;
>> + pa_pmu->sccl_id = -1;
>> pa_pmu->base = devm_platform_ioremap_resource(pdev, 0);
>> if (IS_ERR(pa_pmu->base)) {
>> @@ -399,13 +399,9 @@ static int hisi_pa_pmu_probe(struct
>> platform_device *pdev)
>> ret = hisi_pa_pmu_dev_probe(pdev, pa_pmu);
>> if (ret)
>> return ret;
>> - /*
>> - * PA is attached in SICL and the CPU core is chosen to manage this
>> - * PMU which is the nearest SCCL, while its SCCL_ID is greater than
>> - * one with the SICL_ID.
>> - */
>> +
>> name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "hisi_sicl%u_pa%u",
>> - pa_pmu->sccl_id - 1, pa_pmu->index_id);
>> + pa_pmu->sicl_id, pa_pmu->index_id);
>> if (!name)
>> return -ENOMEM;
>> diff --git a/drivers/perf/hisilicon/hisi_uncore_pmu.c
>> b/drivers/perf/hisilicon/hisi_uncore_pmu.c
>> index a738aeab5c04..31930166c34b 100644
>> --- a/drivers/perf/hisilicon/hisi_uncore_pmu.c
>> +++ b/drivers/perf/hisilicon/hisi_uncore_pmu.c
>> @@ -458,6 +458,10 @@ static bool hisi_pmu_cpu_is_associated_pmu(struct
>> hisi_pmu *hisi_pmu)
>> {
>> int sccl_id, ccl_id;
>> + /* If SCCL_ID is -1, the PMU is in SICL and can be associated
>> this PMU with any CPU */
>
> too long, just have "The PMU is in a SICL and has no CPU affinity"
ok, will fix this.
Thanks,
Qi
>
>> + if (hisi_pmu->sccl_id == -1)
>> + return true;
>> +
>> if (hisi_pmu->ccl_id == -1) {
>> /* If CCL_ID is -1, the PMU only shares the same SCCL */
>> hisi_read_sccl_and_ccl_id(&sccl_id, NULL);
>> diff --git a/drivers/perf/hisilicon/hisi_uncore_pmu.h
>> b/drivers/perf/hisilicon/hisi_uncore_pmu.h
>> index 7f5841d6f592..96eeddad55ff 100644
>> --- a/drivers/perf/hisilicon/hisi_uncore_pmu.h
>> +++ b/drivers/perf/hisilicon/hisi_uncore_pmu.h
>> @@ -81,6 +81,7 @@ struct hisi_pmu {
>> struct device *dev;
>> struct hlist_node node;
>> int sccl_id;
>> + int sicl_id;
>> int ccl_id;
>> void __iomem *base;
>> /* the ID of the PMU modules */
>
> .