| Message ID | 20211116113536.69758-1-jean-philippe@linaro.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | perf/smmuv3: Support devicetree | expand |
On 2021-11-16 11:35, Jean-Philippe Brucker wrote: > Add devicetree binding for the SMMUv3 PMU, called Performance Monitoring > Counter Group (PMCG) in the spec. Each SMMUv3 implementation can have > multiple independent PMCGs, for example one for the Translation Control > Unit (TCU) and one per Translation Buffer Unit (TBU). > > I previously sent the binding as reply to Jay Chen's thread implementing > device tree support [1]. This posting addresses the comments from that > thread. Ha, I'd also resurrected this and was planning to post it at some point this week[0] - you should have said :) > Patch 1 adds two compatible strings. "arm,smmu-v3-pmcg" is common to all > PMCGs. "hisilicon,smmu-v3-pmcg-hip08" allows to support the same quirk > as IORT for that implementation (see patch 2). We'll probably want to > also introduce compatible strings for each implementation that has > additional perf events. For example the MMU-600 implementation has > different events for TCU and TBU PMCGs [2], but both components have the > same device IDs. So the driver could differentiate them if they had two > distinct compatible strings such as "arm,mmu-600-pmcg-tbu" and > "arm,mmu-600-pmcg-tcu". Actually it only needs a general MMU-600 compatible, since once you know it's an Arm Ltd. implementation, you can assume the pattern for the IMP_DEF ID registers to figure out the rest. Robin. [0] https://gitlab.arm.com/linux-arm/linux-rm/-/commits/smmu-pmcg > The series doesn't deal with this because for testing I use a software > model which only implements architected events. I do not include DTS > change for that platform because enabling PMCGs requires an additional > model option. See my branch smmu/pmu-dt [3] for details. > > [1] https://lore.kernel.org/all/20200707150114.GC159413@myrica/ > [2] https://developer.arm.com/documentation/100310/0202/Functional-description/Operation/Performance-Monitoring-Unit > [3] https://jpbrucker.net/git/linux/log/?h=smmu/pmu-dt > > Jean-Philippe Brucker (2): > dt-bindings: Add Arm SMMUv3 PMCG binding > perf/smmuv3: Add devicetree support > > .../bindings/iommu/arm,smmu-v3-pmcg.yaml | 67 +++++++++++++++++++ > drivers/perf/arm_smmuv3_pmu.c | 25 ++++++- > 2 files changed, 90 insertions(+), 2 deletions(-) > create mode 100644 Documentation/devicetree/bindings/iommu/arm,smmu-v3-pmcg.yaml >
On Tue, Nov 16, 2021 at 12:02:47PM +0000, Robin Murphy wrote: > On 2021-11-16 11:35, Jean-Philippe Brucker wrote: > > Add devicetree binding for the SMMUv3 PMU, called Performance Monitoring > > Counter Group (PMCG) in the spec. Each SMMUv3 implementation can have > > multiple independent PMCGs, for example one for the Translation Control > > Unit (TCU) and one per Translation Buffer Unit (TBU). > > > > I previously sent the binding as reply to Jay Chen's thread implementing > > device tree support [1]. This posting addresses the comments from that > > thread. > > Ha, I'd also resurrected this and was planning to post it at some point this > week[0] - you should have said :) Ah sorry about that, I just resent because there was some demand for it at Linaro > > Patch 1 adds two compatible strings. "arm,smmu-v3-pmcg" is common to all > > PMCGs. "hisilicon,smmu-v3-pmcg-hip08" allows to support the same quirk > > as IORT for that implementation (see patch 2). We'll probably want to > > also introduce compatible strings for each implementation that has > > additional perf events. For example the MMU-600 implementation has > > different events for TCU and TBU PMCGs [2], but both components have the > > same device IDs. So the driver could differentiate them if they had two > > distinct compatible strings such as "arm,mmu-600-pmcg-tbu" and > > "arm,mmu-600-pmcg-tcu". > > Actually it only needs a general MMU-600 compatible, since once you know > it's an Arm Ltd. implementation, you can assume the pattern for the IMP_DEF > ID registers to figure out the rest. It might be an error in the MMU-600 spec specifically, both TBU and TCU PMU registers have a 0x83 PIDR0, where I think the TBU should be 0x84 (the revC model uses that value). It's possible that the implementation actually has 0x84 instead. Thanks, Jean
On 2021-11-16 15:42, Jean-Philippe Brucker wrote: > On Tue, Nov 16, 2021 at 12:02:47PM +0000, Robin Murphy wrote: >> On 2021-11-16 11:35, Jean-Philippe Brucker wrote: >>> Add devicetree binding for the SMMUv3 PMU, called Performance Monitoring >>> Counter Group (PMCG) in the spec. Each SMMUv3 implementation can have >>> multiple independent PMCGs, for example one for the Translation Control >>> Unit (TCU) and one per Translation Buffer Unit (TBU). >>> >>> I previously sent the binding as reply to Jay Chen's thread implementing >>> device tree support [1]. This posting addresses the comments from that >>> thread. >> >> Ha, I'd also resurrected this and was planning to post it at some point this >> week[0] - you should have said :) > > Ah sorry about that, I just resent because there was some demand for it at > Linaro Heh, no worries - it's not like you were even CC'ed on the thread where I only mentioned I *might* do it. Can I get away with being cheeky and just saying that my review comments are the diff between my branch and yours, I wonder... >>> Patch 1 adds two compatible strings. "arm,smmu-v3-pmcg" is common to all >>> PMCGs. "hisilicon,smmu-v3-pmcg-hip08" allows to support the same quirk >>> as IORT for that implementation (see patch 2). We'll probably want to >>> also introduce compatible strings for each implementation that has >>> additional perf events. For example the MMU-600 implementation has >>> different events for TCU and TBU PMCGs [2], but both components have the >>> same device IDs. So the driver could differentiate them if they had two >>> distinct compatible strings such as "arm,mmu-600-pmcg-tbu" and >>> "arm,mmu-600-pmcg-tcu". >> >> Actually it only needs a general MMU-600 compatible, since once you know >> it's an Arm Ltd. implementation, you can assume the pattern for the IMP_DEF >> ID registers to figure out the rest. > > It might be an error in the MMU-600 spec specifically, both TBU and TCU > PMU registers have a 0x83 PIDR0, where I think the TBU should be 0x84 (the > revC model uses that value). It's possible that the implementation > actually has 0x84 instead. Yup, it's a mistake in the TRM. I just checked a real MMU-600 and the PMU PIDRs match the main TCU/TBU PIDRs as expected. At least the MMU-700 docs haven't repeated the same error. Cheers, Robin.
On Tue, Nov 16, 2021 at 05:00:14PM +0000, Robin Murphy wrote: > On 2021-11-16 15:42, Jean-Philippe Brucker wrote: > > On Tue, Nov 16, 2021 at 12:02:47PM +0000, Robin Murphy wrote: > > > On 2021-11-16 11:35, Jean-Philippe Brucker wrote: > > > > Add devicetree binding for the SMMUv3 PMU, called Performance Monitoring > > > > Counter Group (PMCG) in the spec. Each SMMUv3 implementation can have > > > > multiple independent PMCGs, for example one for the Translation Control > > > > Unit (TCU) and one per Translation Buffer Unit (TBU). > > > > > > > > I previously sent the binding as reply to Jay Chen's thread implementing > > > > device tree support [1]. This posting addresses the comments from that > > > > thread. > > > > > > Ha, I'd also resurrected this and was planning to post it at some point this > > > week[0] - you should have said :) > > > > Ah sorry about that, I just resent because there was some demand for it at > > Linaro > > Heh, no worries - it's not like you were even CC'ed on the thread where I > only mentioned I *might* do it. > > Can I get away with being cheeky and just saying that my review comments are > the diff between my branch and yours, I wonder... Sure, that works for me, I'll send a v2 this week or so Thanks, Jean