From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.0 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 2CB36C4346E for ; Mon, 21 Sep 2020 18:37:25 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id C454A207BC for ; Mon, 21 Sep 2020 18:37:24 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="v//h5y/G"; dkim=fail reason="signature verification failed" (1024-bit key) header.d=kernel.org header.i=@kernel.org header.b="kBi1DkUk" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org C454A207BC Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Transfer-Encoding: Content-Type:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References:Message-ID: Subject:To:From:Date:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=pw/MkSQupEBDQ98Tmf2a8u65QPYkeYjElAeBjIjqPRw=; b=v//h5y/Gi5SRfpCIDBXit9qHu cSAZKdAoL2JnLOmDsKzC5Gasc0uzuzYWN3x6MPRA68PsVCp7cW82SWD8sdbHWdCuV8g566CA+xkx8 8J+RhbNNPnD89gcbbnZ9e4ukviUOQFOk9Im+0MD/GF7PgSI6GwpBVOHAdpX5mZQby6KgIkbnHVSs8 zVhiCjV07LKZkJTZ/yRGt9/3o8EtjrcYsAU2bkJK7wmDD/kILM/NBqMVcJcnlSfoL7YEBs0X1kuzP BIMcM7scQ/N3bwWVrsRZR+FP+//9oDQAiIM7bGG7yw6o9DElUqsodMb0IFVTi8HA+sUxXQDQGHdzl GMIzp0zPA==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kKQfb-0003Bb-QB; Mon, 21 Sep 2020 18:36:11 +0000 Received: from mail.kernel.org ([198.145.29.99]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1kKQfX-0003AX-S2 for linux-arm-kernel@lists.infradead.org; Mon, 21 Sep 2020 18:36:10 +0000 Received: from willie-the-truck (236.31.169.217.in-addr.arpa [217.169.31.236]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 08D9A207BC; Mon, 21 Sep 2020 18:36:05 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1600713367; bh=oIWNa2bvQa4VgKHHtNgrsN0st3L5HMioVqgv04biicE=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=kBi1DkUkhY8WeiOqOVMurQdLAsHJ4Y0M1PKAkyiEd7hZkJocofJzB65Qe0tob9crZ 2IBLLWRExPJKxdUQBuG4MDk1dstrnrXXkjMU+CYPSZBPsIXQl7dllsayUnyglULm1K F35uMTGu7xW5ShhsbVPihSJBkETvKd+sQEuT/cjA= Date: Mon, 21 Sep 2020 19:36:02 +0100 From: Will Deacon To: Joakim Zhang Subject: Re: [PATCH V2] perf/imx_ddr: Add stop event counters support for i.MX8MP Message-ID: <20200921183602.GI3141@willie-the-truck> References: <1599562054-1930-1-git-send-email-qiangqing.zhang@nxp.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <1599562054-1930-1-git-send-email-qiangqing.zhang@nxp.com> User-Agent: Mutt/1.10.1 (2018-07-13) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200921_143608_036327_A16FADDB X-CRM114-Status: GOOD ( 31.09 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: mark.rutland@arm.com, robin.murphy@arm.com, linux-imx@nxp.com, linux-arm-kernel@lists.infradead.org Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Tue, Sep 08, 2020 at 06:47:34PM +0800, Joakim Zhang wrote: > DDR Perf driver only supports free-running event counters(counter1/2/3) > now, this patch adds support for stop event counters. > > Legacy SoCs: > Cycle counter(counter0) is a special counter, only count cycles. When > cycle counter overflow, it will lock all counters and generate an > interrupt. In ddr_perf_irq_handler, disable cycle counter then all > counters would stop at the same time, update all counters' count, then > enable cycle counter that all counters count again. During this process, > only clear cycle counter, no need to clear event counters since they are > free-running counters. They would continue counting after overflow and > do/while loop from ddr_perf_event_update can handle event counters > overflow case. > > i.MX8MP: > Almost all is the same as legacy SoCs, the only difference is that, event > counters are not free-running any more. Like cycle counter, when event > counters overflow, they would stop counting unless clear the counter, > and no interrupt generate for event counters. So we should clear event > counters that let them re-count when cycle counter overflow, which ensure > event counters will not lose data. > > This patch adds stop event counters support which would be compatible to > free-running event counters. > > Signed-off-by: Joakim Zhang > --- > ChangeLogs: > V1->V2: > * clear event counters in update function, instead of irq > handler, so remove spinlock. > --- > drivers/perf/fsl_imx8_ddr_perf.c | 68 ++++++++++++++++++++++---------- > 1 file changed, 48 insertions(+), 20 deletions(-) > > diff --git a/drivers/perf/fsl_imx8_ddr_perf.c b/drivers/perf/fsl_imx8_ddr_perf.c > index 90884d14f95f..c0f0adfcac06 100644 > --- a/drivers/perf/fsl_imx8_ddr_perf.c > +++ b/drivers/perf/fsl_imx8_ddr_perf.c > @@ -361,25 +361,6 @@ static int ddr_perf_event_init(struct perf_event *event) > return 0; > } > > - > -static void ddr_perf_event_update(struct perf_event *event) > -{ > - struct ddr_pmu *pmu = to_ddr_pmu(event->pmu); > - struct hw_perf_event *hwc = &event->hw; > - u64 delta, prev_raw_count, new_raw_count; > - int counter = hwc->idx; > - > - do { > - prev_raw_count = local64_read(&hwc->prev_count); > - new_raw_count = ddr_perf_read_counter(pmu, counter); > - } while (local64_cmpxchg(&hwc->prev_count, prev_raw_count, > - new_raw_count) != prev_raw_count); > - > - delta = (new_raw_count - prev_raw_count) & 0xFFFFFFFF; > - > - local64_add(delta, &event->count); > -} > - > static void ddr_perf_counter_enable(struct ddr_pmu *pmu, int config, > int counter, bool enable) > { > @@ -404,6 +385,52 @@ static void ddr_perf_counter_enable(struct ddr_pmu *pmu, int config, > } > } > > +static bool ddr_perf_counter_overflow(struct ddr_pmu *pmu, int counter) > +{ > + int val; Do you really need this to be signed? > + val = readl_relaxed(pmu->base + counter * 4 + COUNTER_CNTL); > + > + return val & CNTL_OVER ? true : false; Just return val & CNTL_OVER. > +} > + > +static void ddr_perf_event_update(struct perf_event *event) > +{ > + struct ddr_pmu *pmu = to_ddr_pmu(event->pmu); > + struct hw_perf_event *hwc = &event->hw; > + u64 delta, prev_raw_count, new_raw_count; > + int counter = hwc->idx; > + int ret; > + > + if (counter == EVENT_CYCLES_COUNTER) { > + do { > + prev_raw_count = local64_read(&hwc->prev_count); > + new_raw_count = ddr_perf_read_counter(pmu, counter); > + } while (local64_cmpxchg(&hwc->prev_count, prev_raw_count, > + new_raw_count) != prev_raw_count); > + > + delta = (new_raw_count - prev_raw_count) & 0xFFFFFFFF; > + > + local64_add(delta, &event->count); Why do we treat the cycle counter so differently here? > + } else { > + /* > + * For legacy SoCs: event counters continue counting when overflow, > + * no need to clear the counter. > + * For new SoCs: event counters stop counting when overflow, need > + * clear counter to let it count again. > + */ > + ret = ddr_perf_counter_overflow(pmu, counter); > + if (ret) > + dev_warn(pmu->dev, "Event Counter%d overflow happened, data incorrect!!\n", counter); I don't understand this message: if the data is incorrect, why do we need to handle overflow at all/, rather than putting the event into an error state? Will _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel