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 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 2EBB0C4828F for ; Thu, 8 Feb 2024 10:51:13 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Message-ID:Subject:CC: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=9AAWsje8omHbzxG9GeGCP+FwfXsSbEkuY6GI+Y9gF0s=; b=3qnk9apvp9fAdT P//IoUhvzAPF49GIQOBB+/1jfJg5MzhVp4raXX7hsAZf23Rt5z3eIEphUJvfWDJuK1x6qdYsW38u/ cASPwLnen5Tr0A47TjRFAqvUDctpAn29KFbn9tX33qJWdBH3GWJIaMQQ4QwbJN30eb4M6GB4QGsPW AsDEiCb+5t1Kiz7j6T49Lf62vnVOTlAhoA0Bq87wCCTtDh7i4p2mRwRNyoTFCXXN41mW2NPxvWTct yh7jp5P569HuMSLYXsAahSZyaY+46lSP6Bm57g8HPR1kGTQ/RXFbR9KOan96E+tZxABspPJQ811Ud 3KS3zLuCiryOiFdzucUg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1rY1zd-0000000DQUi-2anT; Thu, 08 Feb 2024 10:50:57 +0000 Received: from frasgout.his.huawei.com ([185.176.79.56]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1rY1za-0000000DQU4-3Ma0 for linux-arm-kernel@lists.infradead.org; Thu, 08 Feb 2024 10:50:56 +0000 Received: from mail.maildlp.com (unknown [172.18.186.31]) by frasgout.his.huawei.com (SkyGuard) with ESMTP id 4TVtwg0HC7z6K8yL; Thu, 8 Feb 2024 18:47:31 +0800 (CST) Received: from lhrpeml500005.china.huawei.com (unknown [7.191.163.240]) by mail.maildlp.com (Postfix) with ESMTPS id DCC94140595; Thu, 8 Feb 2024 18:50:48 +0800 (CST) Received: from localhost (10.202.227.76) by lhrpeml500005.china.huawei.com (7.191.163.240) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.35; Thu, 8 Feb 2024 10:50:48 +0000 Date: Thu, 8 Feb 2024 10:50:47 +0000 From: Jonathan Cameron To: Junhao He CC: , , , , , Subject: Re: [PATCH] drivers/perf: hisi: Enable HiSilicon Erratum 162700402 quirk for UC PMU Message-ID: <20240208105047.000023c7@Huawei.com> In-Reply-To: <20240207094245.34195-1-hejunhao3@huawei.com> References: <20240207094245.34195-1-hejunhao3@huawei.com> Organization: Huawei Technologies Research and Development (UK) Ltd. X-Mailer: Claws Mail 4.1.0 (GTK 3.24.33; x86_64-w64-mingw32) MIME-Version: 1.0 X-Originating-IP: [10.202.227.76] X-ClientProxiedBy: lhrpeml100003.china.huawei.com (7.191.160.210) To lhrpeml500005.china.huawei.com (7.191.163.240) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240208_025055_184562_1FD4FA48 X-CRM114-Status: GOOD ( 24.13 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , 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 Wed, 7 Feb 2024 17:42:45 +0800 Junhao He wrote: > HiSilicon UC PMU v2 suffers the erratum 162700402 that the PMU counter > cannot be set due to the lack of clock under power saving mode. This will > lead to error or inaccurate counts. The clock can be enabled by the PMU > global enabling control. > > This patch tries to fix this by set the UC PMU enable before set event > period to turn on the clock, and then restore the UC PMU configuration. > The counter register can hold its value without a clock. > > Signed-off-by: Junhao He Hi. Some very minor comments about the comments inline. Jonathan > --- > drivers/perf/hisilicon/hisi_uncore_uc_pmu.c | 40 ++++++++++++++++++++- > 1 file changed, 39 insertions(+), 1 deletion(-) > > diff --git a/drivers/perf/hisilicon/hisi_uncore_uc_pmu.c b/drivers/perf/hisilicon/hisi_uncore_uc_pmu.c > index 636fb79647c8..8e7a9e1f419a 100644 > --- a/drivers/perf/hisilicon/hisi_uncore_uc_pmu.c > +++ b/drivers/perf/hisilicon/hisi_uncore_uc_pmu.c > @@ -287,10 +287,48 @@ static u64 hisi_uc_pmu_read_counter(struct hisi_pmu *uc_pmu, > return readq(uc_pmu->base + HISI_UC_CNTR_REGn(hwc->idx)); > } > > -static void hisi_uc_pmu_write_counter(struct hisi_pmu *uc_pmu, > +static bool hisi_uc_pmu_get_glb_en_state(struct hisi_pmu *uc_pmu) > +{ > + u32 val; > + > + val = readl(uc_pmu->base + HISI_UC_EVENT_CTRL_REG); > + return !!FIELD_GET(HISI_UC_EVENT_GLB_EN, val); > +} > + > +static void hisi_uc_pmu_write_counter_quirk_hip09(struct hisi_pmu *uc_pmu, > struct hw_perf_event *hwc, u64 val) > { > + bool enable = hisi_uc_pmu_get_glb_en_state(uc_pmu); > + > + /* Set the UC PMU enable to turn on the clock. */ Comment from below here but adjusted to say which path has the device already enabled. > + if (!enable) > + hisi_uc_pmu_start_counters(uc_pmu); > + > writeq(val, uc_pmu->base + HISI_UC_CNTR_REGn(hwc->idx)); > + > + /* > + * The counter register can hold its value without a clock. We need > + * restore the UC PMU configuration. The irq handler will also call > + * the function to set period. At this time, PMU is still enabled and > + * we cannot directly disable the PMU. I think the comment is more relevant above... > + */ > + if (!enable) > + hisi_uc_pmu_stop_counters(uc_pmu); > +} > + > +static void hisi_uc_pmu_write_counter(struct hisi_pmu *uc_pmu, > + struct hw_perf_event *hwc, u64 val) > +{ > + /* > + * HiSilicon UC PMU v2 suffers the erratum 162700402 that the PMU > + * counter cannot be set due to the lack of clock under power saving > + * mode. This will lead to error or inaccurate counts. The clock can > + * be enabled by the PMU global enabling control. I'd move the comment to next to the quirk function so that people can immediately see what is being done. Down here we just need to know there is a quirk. > + */ > + if (uc_pmu->identifier == HISI_PMU_V2) > + hisi_uc_pmu_write_counter_quirk_hip09(uc_pmu, hwc, val); > + else > + writeq(val, uc_pmu->base + HISI_UC_CNTR_REGn(hwc->idx)); > } > > static void hisi_uc_pmu_enable_counter_int(struct hisi_pmu *uc_pmu, _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel