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.8 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED 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 E1046C2D0E4 for ; Tue, 24 Nov 2020 17:52:38 +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 2DEAC20757 for ; Tue, 24 Nov 2020 17:52:37 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="dXGOk0OQ"; dkim=fail reason="signature verification failed" (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b="eQGL7WIn" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 2DEAC20757 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linaro.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=8CV8A6eKHKuAGXeGCmQFLxRb2BgBfJIAV1ILP+y/Q5g=; b=dXGOk0OQufiq6BdBk3GlW0Yg8 6DLZdCgYek14ar/rtxhSxnideqEeWLkMTl8dhTJUcGcfk/3NawZ7yWrD9gtTqDKepRskgaiI47f/U ErWVMNLqfQElzhrjiGs0EyPgt80Fv/seBhcYAGc6RGj04REch7PNA8qbHuvIGQg1UxJx8Zm9KY1dC VFom2+GtSIIyuBauN+ulX8bZjBtJyAPGx2mhuVcgMDs2ajuySmvor2aEV20cp3bznyHrWDV+3mXUx 6L82axP8MbhRqqJ5xmUuwmUp8NwiNbEYCT6w5VK9FBrNYexhnu4VX8nm3Uj5xCCF/46Fk5QTVASly 0Pp3Txmww==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1khcT8-0006QV-Re; Tue, 24 Nov 2020 17:51:10 +0000 Received: from mail-pg1-x541.google.com ([2607:f8b0:4864:20::541]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1khcT6-0006Py-EU for linux-arm-kernel@lists.infradead.org; Tue, 24 Nov 2020 17:51:09 +0000 Received: by mail-pg1-x541.google.com with SMTP id t3so4275471pgi.11 for ; Tue, 24 Nov 2020 09:51:07 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=hwR/fBUhr/qmn50v2UeaTs5CjkpD+xDMtYGudWYTfFI=; b=eQGL7WInhNYf1tr54O3bsDTHngV3Xsli4BIx4CdLX3EtY+ryi63uVjMiepNSmDku/J bSqGRWJQxCeF3/baec5xTRNjJQfeapbKDZquBBaG1pE05ewb1JcqXYWbHraLXbhdRpZx A1XQ7IP1pSM0T1I5h9K4GzNSvplF+HvsfxGqjxxbn7NPE/aOOwo0Uc5x+LZDMwXjdX/4 bI/JFFYOYXwWp4/eJ86DjZ3HhAureVV0R21YSlx+YVvCrWXB5jdGKlaDGuTQP+yI6Z06 0XRv5RUMhy99xuAdmRUWGsaVy4A5xFOuhUoCFS31HxCudSwb+3wnA+yhsinPNAxPP+iF A3mA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=hwR/fBUhr/qmn50v2UeaTs5CjkpD+xDMtYGudWYTfFI=; b=aGbE90oCksUCB4NaNhRifIGloHbMkkRMOxJeqZZ9/Q1xU0jqqY8U9hXft9IhQMdvRc th8Fm8ahCgXTsI8jksB1WzXbtK4GCz3ppE0yQPbvtRR+SZlFKJP4emh+xOnTGBv1hjjm UNUvN7tl4SD2lrUfGihOyzPwPKlo3yVAvrCYyAv/SFRyWqEJAxiyFheJ/f2nXfZ9MvR4 JmBn8ABwQ254AHXNkhSn/01FsgdsGfKz+JzKAT6G8yH1iZj3qf6WwtbfXglDD5k1dZUa bRVxOXYtslRdMWtbTIBIo4+eE3LjE60jAuMZmz5MMhIKmLd69ZrtLxI/oGbYyZF4SEsm q9vA== X-Gm-Message-State: AOAM532tjCJLlEfVAl2Gfj8kTUAxQWVDpk0m/TwNSL3C4XRcy8LIN+6O 9ENBx94hJCjomfqtnBulnqEpiQ== X-Google-Smtp-Source: ABdhPJyop1vc0FnmSklP7FZUX6fFNegXfaHoZdoRhOYxepB8ZrKNZFuxTy1nng4zinVXdaoPnHzcQw== X-Received: by 2002:a62:88d4:0:b029:198:c01:f2a9 with SMTP id l203-20020a6288d40000b02901980c01f2a9mr4919984pfd.45.1606240266135; Tue, 24 Nov 2020 09:51:06 -0800 (PST) Received: from xps15 (S0106889e681aac74.cg.shawcable.net. [68.147.0.187]) by smtp.gmail.com with ESMTPSA id r11sm3919559pjd.52.2020.11.24.09.51.04 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 24 Nov 2020 09:51:05 -0800 (PST) Date: Tue, 24 Nov 2020 10:51:03 -0700 From: Mathieu Poirier To: Mike Leach Subject: Re: [RFC PATCH v3 6/9] coresight: config: Add preloaded configurations Message-ID: <20201124175103.GA555140@xps15> References: <20201030175655.30126-1-mike.leach@linaro.org> <20201030175655.30126-7-mike.leach@linaro.org> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20201030175655.30126-7-mike.leach@linaro.org> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20201124_125108_530256_8D4601E2 X-CRM114-Status: GOOD ( 34.35 ) 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: yabinc@google.com, coresight@lists.linaro.org, linux-arm-kernel@lists.infradead.org, suzuki.poulose@arm.com 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 Fri, Oct 30, 2020 at 05:56:52PM +0000, Mike Leach wrote: > Preload set of configurations. > > This patch creates a small set of preloaded configurations and features > that are available immediately after coresight has been initialised. > > The current set provides a strobing feature for ETMv4, that creates a > periodic sampling of trace by switching trace generation on and off > using counters in the ETM. > > A configuration called "autofdo" is also provided that uses the 'strobing' > feature and provides a couple of preset values, selectable on the perf > command line. > > Signed-off-by: Mike Leach > --- > drivers/hwtracing/coresight/Makefile | 3 +- > .../coresight/coresight-cfg-preload.c | 160 ++++++++++++++++++ > drivers/hwtracing/coresight/coresight-core.c | 6 + > .../hwtracing/coresight/coresight-syscfg.h | 1 + > 4 files changed, 169 insertions(+), 1 deletion(-) > create mode 100644 drivers/hwtracing/coresight/coresight-cfg-preload.c > > diff --git a/drivers/hwtracing/coresight/Makefile b/drivers/hwtracing/coresight/Makefile > index ea544206204d..9de5586cfd1a 100644 > --- a/drivers/hwtracing/coresight/Makefile > +++ b/drivers/hwtracing/coresight/Makefile > @@ -4,7 +4,8 @@ > # > obj-$(CONFIG_CORESIGHT) += coresight.o > coresight-y := coresight-core.o coresight-etm-perf.o coresight-platform.o \ > - coresight-sysfs.o coresight-syscfg.o coresight-config.o > + coresight-sysfs.o coresight-syscfg.o coresight-config.o \ > + coresight-cfg-preload.o > obj-$(CONFIG_CORESIGHT_LINK_AND_SINK_TMC) += coresight-tmc.o > coresight-tmc-y := coresight-tmc-core.o coresight-tmc-etf.o \ > coresight-tmc-etr.o > diff --git a/drivers/hwtracing/coresight/coresight-cfg-preload.c b/drivers/hwtracing/coresight/coresight-cfg-preload.c > new file mode 100644 > index 000000000000..0975d64a1d9e > --- /dev/null > +++ b/drivers/hwtracing/coresight/coresight-cfg-preload.c I think there needs to be a separation between the function of preloading configurations and the preconfigurations themselves. Ultimately having one .c file for each preconfiguration would be optimal. That will open the way for a future feature where users get to select which preconfiguration they want to see included. > @@ -0,0 +1,160 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright(C) 2020 Linaro Limited. All rights reserved. > + * Author: Mike Leach > + */ > + > +#include "coresight-config.h" > +#include "coresight-syscfg.h" > +#include "coresight-etm4x-cfg.h" > + > +/* preload configurations and features */ > + > +/* preload in features for ETMv4 */ > + > +/* strobe feature */ > + > +/* register defines */ > +#define STRB_REG_CTR (CS_CFG_REG_RESOURCE | ETM4_CFG_RES_CTR) > +#define STRB_REG_CTR_RB (STRB_REG_CTR | CS_CFG_REG_VAL_SAVE) > +#define STRB_REG_CTR_PRM (STRB_REG_CTR | CS_CFG_REG_VAL_PARAM) > +#define STRB_REG_SEQ (CS_CFG_REG_RESOURCE | ETM4_CFG_RES_SEQ) > +#define STRB_REG_SEL (CS_CFG_REG_RESOURCE | ETM4_CFG_RES_SEL) > +#define STRB_REG_VI (CS_CFG_REG_STD | CS_CFG_REG_VAL_MASK) > + This is the 3rd iteration of this set that I review. Along the way I have come to understand most of the things that are presented. The above and all the information packed in .flags is one concept that did not get better as time went by. I have already remarked on the need to split the register address from the flags themselves. I think the remaining flags need further simplification. Either some options are removed or a different way to present the information is required. Otherwise I fear people will not be able to write their own preconfigurations. > +static struct cscfg_parameter_desc strobe_params[] = { > + { > + .name = "window", > + .value = 5000, > + }, > + { > + .name = "period", > + .value = 10000, > + }, > +}; > + > +static struct cscfg_regval strobe_regs[] = { > + /* resource selectors */ > + { > + .flags = STRB_REG_SEL | TRCRSCTLRn(2), > + .val32 = 0x20001, > + }, > + { > + .flags = STRB_REG_SEQ | TRCRSCTLRn(3), > + .val32 = 0x20002, > + }, > + /* strobe window counter 0 - reload from param 0 */ > + { > + .flags = STRB_REG_CTR_RB | TRCCNTVRn(0), > + }, > + { > + .flags = STRB_REG_CTR_PRM | TRCCNTRLDVRn(0), > + .val32 = 0, > + }, > + { > + .flags = STRB_REG_CTR | TRCCNTCTLRn(0), > + .val32 = 0x10001, > + }, > + /* strobe period counter 1 - reload from param 1 */ > + { > + .flags = STRB_REG_CTR_RB | TRCCNTVRn(1), > + }, > + { > + .flags = STRB_REG_CTR_PRM | TRCCNTRLDVRn(1), > + .val32 = 1, > + }, > + { > + .flags = STRB_REG_CTR | TRCCNTCTLRn(1), > + .val32 = 0x8102, > + }, > + /* sequencer */ > + { > + .flags = STRB_REG_SEQ | TRCSEQEVRn(0), > + .val32 = 0x0081, > + }, > + { > + .flags = STRB_REG_SEQ | TRCSEQEVRn(1), > + .val32 = 0x0000, > + }, > + /* view-inst */ > + { > + .flags = STRB_REG_VI | TRCVICTLR, > + .val32 = 0x0003, > + .mask32 = 0x0003, > + }, > + /* end of regs */ > +}; > + > +static struct cscfg_feature_desc strobe = { > + .name = "strobing", > + .brief = "Generate periodic trace capture windows.\n" > + "parameter \'window\': a number of CPU cycles (W)\n" > + "parameter \'period\': trace enabled for W cycles every period x W cycles\n", > + .match_flags = CS_CFG_MATCH_CLASS_SRC_ETM4, > + .nr_params = ARRAY_SIZE(strobe_params), > + .params = strobe_params, > + .nr_regs = ARRAY_SIZE(strobe_regs), > + .regs = strobe_regs, > +}; > + > +static struct cscfg_feature_desc *preload_feats[] = { > + &strobe, > + 0 > +}; > + > +/* create an autofdo configuration */ > + > +/* we will provide 9 sets of preset parameter values */ > +#define AFDO_NR_PRESETS 9 > +/* the total number of parameters in used features */ > +#define AFDO_NR_PARAM_SUM ARRAY_SIZE(strobe_params) > + > +#define AFDO_MATCH_STROBING (CS_CFG_MATCH_INST_ANY | CS_CFG_MATCH_CLASS_SRC_ETM4) > + > +static struct cscfg_config_feat_ref afdo_refs[] = { > + { > + .name = "strobing", > + .nr_params = ARRAY_SIZE(strobe_params), > + .match = { > + .match_flags = AFDO_MATCH_STROBING, > + }, > + }, > +}; > + > +/* > + * set of presets leaves strobing window constant while varying period to allow > + * experimentation with mark / space ratios for various workloads > + */ > +static u64 afdo_presets[AFDO_NR_PRESETS][AFDO_NR_PARAM_SUM] = { > + { 5000, 2 }, > + { 5000, 4 }, > + { 5000, 8 }, > + { 5000, 16 }, > + { 5000, 64 }, > + { 5000, 128 }, > + { 5000, 512 }, > + { 5000, 1024 }, > + { 5000, 4096 }, > +}; > + > +static struct cscfg_config_desc afdo = { > + .name = "autofdo", > + .brief = "Setup ETMs with strobing for autofdo\n" > + "Supplied presets allow experimentation with mark-space ratio for various loads\n", > + .nr_refs = ARRAY_SIZE(afdo_refs), > + .refs = afdo_refs, > + .nr_presets = AFDO_NR_PRESETS, > + .nr_total_params = AFDO_NR_PARAM_SUM, > + .presets = &afdo_presets[0][0], > +}; > + The current layout works if we have a single preconfiguration but doesn't scale as soon as we need to add a new one. I think the above should be in coresight-afdo.c. That way other preconfiguration can be added to preload_cfgs[] without modifying anything else than the array declaration itself. > +static struct cscfg_config_desc *preload_cfgs[] = { > + &afdo, > + 0 > +}; > + > +/* preload called with mutex locked */ > +int cscfg_preload(void) > +{ > + return cscfg_load_config_sets(preload_cfgs, preload_feats); > +} > diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c > index 481d2b7b6b6f..dca84b3f5fca 100644 > --- a/drivers/hwtracing/coresight/coresight-core.c > +++ b/drivers/hwtracing/coresight/coresight-core.c > @@ -1680,9 +1680,15 @@ static int __init coresight_init(void) > > /* initialise the coresight syscfg API */ > ret = cscfg_init(); > + if (ret) > + goto exit_perf_close; > + > + /* preload builtin configurations */ > + ret = cscfg_preload(); > if (!ret) > return 0; > > +exit_perf_close: > etm_perf_exit(); > exit_bus_unregister: > bus_unregister(&coresight_bustype); > diff --git a/drivers/hwtracing/coresight/coresight-syscfg.h b/drivers/hwtracing/coresight/coresight-syscfg.h > index ecf4aac7d712..e8f352599dd7 100644 > --- a/drivers/hwtracing/coresight/coresight-syscfg.h > +++ b/drivers/hwtracing/coresight/coresight-syscfg.h > @@ -45,6 +45,7 @@ struct cscfg_csdev_register { > /* internal core operations for cscfg */ > int __init cscfg_init(void); > void __exit cscfg_exit(void); > +int cscfg_preload(void); > > /* syscfg external API */ > int cscfg_load_config_sets(struct cscfg_config_desc **cfg_descs, > -- > 2.17.1 > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel