RE: [PATCH 2/2] perf/imx_ddr: Add stop counter support for i.MX8MP

From: Joakim Zhang <qiangqing.zhang@nxp.com>
To: Will Deacon <will@kernel.org>
Cc: "mark.rutland@arm.com" <mark.rutland@arm.com>,
	"robin.murphy@arm.com" <robin.murphy@arm.com>,
	dl-linux-imx <linux-imx@nxp.com>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>
Subject: RE: [PATCH 2/2] perf/imx_ddr: Add stop counter support for i.MX8MP
Date: Thu, 21 May 2020 04:57:28 +0000	[thread overview]
Message-ID: <DB8PR04MB679500AE5F752910C9B1024BE6B70@DB8PR04MB6795.eurprd04.prod.outlook.com> (raw)
In-Reply-To: <20200520075140.GB23818@willie-the-truck>

> -----Original Message-----
> From: Will Deacon <will@kernel.org>
> Sent: 2020年5月20日 15:52
> To: Joakim Zhang <qiangqing.zhang@nxp.com>
> Cc: mark.rutland@arm.com; robin.murphy@arm.com; dl-linux-imx
> <linux-imx@nxp.com>; linux-arm-kernel@lists.infradead.org
> Subject: Re: [PATCH 2/2] perf/imx_ddr: Add stop counter support for i.MX8MP
> 
> On Thu, Apr 16, 2020 at 09:51:13AM +0000, Joakim Zhang wrote:
> > Any comments about this issue? Thanks a lot!
> 
> You didn't really answer any of my questions, so I don't really know what to do
> with this.
> 
>   - The locking appears to be broken. Your solution was to remove it
>     entirely.
> 
>   - It appears to be a user visible change and you haven't explained how it
>     continues to work with old userspace
> 
>   - Perf core is not aware of you stopping counters and you haven't said why
>     that's not an issue.
> 
> While these issues are outstanding, I cannot merge the patch. Sorry.

Hi Will,

You are really kind. Sorry for that, sometimes I am not quite understand what you want. I send out this patch, just want to talk with you to find a better solution, you could provide profession opinion.

Actually new SoC has a hardware change:

Old SoC:
Counter0 is a special counter, only count cycles. Counter1-3 are event counters. When counter0 overflow, it will lock all counters and generate an interrupt. In ddr_perf_irq_handler(), disable counter0 then all counter1-3 will stop at the same time, update all counters' count, then enable counter0 that all counters would count again. You can see that when enable counter0 it would clear overflow bit, but counter1-3 are free-running, need not clear it. Do/while() from ddr_perf_event_update() can handle counter1-3 overflow case.

MX8MP:
Almost is same with old SoC, the only different is that, counter1-3 are not free-running now. Like counter0, when counter1-3 are overflow, they would stop counting unless clear their overflow bit. Counter0 overflow occurs at least 4 times as often as other counters, so I want to re-enable counter1-3 then they can re-count again, to ensure that counter1-3 will not lose data. The key is that I need clear counter1-3 in counter0 irq handler.

The count updating would happen at irq handler or perf core(read callback). I add a spinlock to avoid updating counter1-3 while clearing counter1-3, but I am not sure if it needs. Looking forward to your feedbacks, please point out my mistakes. Thanks a lot.

Best Regards,
Joakim Zhang
> Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel