Messages in this thread | | | Subject | Re: [PATCH v3 1/2] drivers/perf: hisi: Associate PMUs in SICL with CPUs online | From | "liuqi (BA)" <> | Date | Wed, 6 Apr 2022 09:37:32 +0800 |
| |
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 <liuqi115@huawei.com> >> --- >> 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 */ > > .
| |