[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAJ9a7VgqGJ=YLG6+ypMnqV9YJ_dMBw6qyhusXdR_NR2=RTis-w@mail.gmail.com>
Date: Thu, 27 Nov 2025 15:48:03 +0000
From: Mike Leach <mike.leach@...aro.org>
To: James Clark <james.clark@...aro.org>
Cc: Suzuki K Poulose <suzuki.poulose@....com>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>, Jonathan Corbet <corbet@....net>,
Leo Yan <leo.yan@....com>, Randy Dunlap <rdunlap@...radead.org>, coresight@...ts.linaro.org,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
linux-doc@...r.kernel.org
Subject: Re: [PATCH v7 12/13] coresight: Allow setting the timestamp interval
Hi James
On Wed, 26 Nov 2025 at 10:57, James Clark <james.clark@...aro.org> wrote:
>
> Timestamps are currently emitted at the maximum rate possible, which is
> much too frequent for most use cases. Set the interval using the value
> from the timestamp field. Granular control is not required, so save
> space in the config by interpreting it as 2 ^ timestamp. And then 4
> bits (0 - 15) is enough to set the interval to be larger than the
> existing SYNC timestamp interval.
>
> No sysfs mode support is needed for this attribute because counter
> generated timestamps are only configured for Perf mode.
>
> Reviewed-by: Leo Yan <leo.yan@....com>
> Tested-by: Leo Yan <leo.yan@....com>
> Signed-off-by: James Clark <james.clark@...aro.org>
> ---
> drivers/hwtracing/coresight/coresight-etm-perf.h | 1 +
> drivers/hwtracing/coresight/coresight-etm4x-core.c | 28 +++++++++++++++-------
> 2 files changed, 20 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.h b/drivers/hwtracing/coresight/coresight-etm-perf.h
> index 24d929428633..128f80bb1443 100644
> --- a/drivers/hwtracing/coresight/coresight-etm-perf.h
> +++ b/drivers/hwtracing/coresight/coresight-etm-perf.h
> @@ -7,6 +7,7 @@
> #ifndef _CORESIGHT_ETM_PERF_H
> #define _CORESIGHT_ETM_PERF_H
>
> +#include <linux/bits.h>
> #include <linux/percpu-defs.h>
> #include "coresight-priv.h"
>
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> index c7bf73c8f2d7..0129b0502726 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> @@ -651,7 +651,7 @@ static void etm4_enable_sysfs_smp_call(void *info)
> * +--------------+
> * |
> * +------v-------+
> - * | Counter x | (reload to 1 on underflow)
> + * | Counter x | (reload to 2 ^ timestamp on underflow)
> * +--------------+
> * |
> * +------v--------------+
> @@ -662,11 +662,25 @@ static void etm4_enable_sysfs_smp_call(void *info)
> * | Timestamp Generator | (timestamp on resource y)
> * +----------------------+
> */
> -static int etm4_config_timestamp_event(struct etmv4_drvdata *drvdata)
> +static int etm4_config_timestamp_event(struct etmv4_drvdata *drvdata,
> + struct perf_event_attr *attr)
> {
> int ctridx;
> int rselector;
> struct etmv4_config *config = &drvdata->config;
> + struct perf_event_attr max_timestamp = {
> + .ATTR_CFG_FLD_timestamp_CFG = U64_MAX,
> + };
> +
> + /* timestamp may be 0 if deprecated_timestamp is used, so make min 1 */
> + u8 ts_level = max(1, ATTR_CFG_GET_FLD(attr, timestamp));
> +
I could be missing something here - but if we have a perf command line:
perf -e cs_etm/timestamp=0/
is this bit not changing that to timestamp=1 regardless? The docs
(patch 13) indicate timestamp=0 to be timestamps off.
This command is used in test_arm_coresight.sh when testing the
combination of options on the CS system.
Mike
> + /*
> + * Disable counter generated timestamps when timestamp == MAX. Leave
> + * only SYNC timestamps.
> + */
> + if (ts_level == ATTR_CFG_GET_FLD(&max_timestamp, timestamp))
> + return 0;
>
> /* No point in trying if we don't have at least one counter */
> if (!drvdata->nr_cntr)
> @@ -704,12 +718,8 @@ static int etm4_config_timestamp_event(struct etmv4_drvdata *drvdata)
> return -ENOSPC;
> }
>
> - /*
> - * Initialise original and reload counter value to the smallest
> - * possible value in order to get as much precision as we can.
> - */
> - config->cntr_val[ctridx] = 1;
> - config->cntrldvr[ctridx] = 1;
> + /* Initialise original and reload counter value. */
> + config->cntr_val[ctridx] = config->cntrldvr[ctridx] = 1 << (ts_level - 1);
>
> /*
> * Trace Counter Control Register TRCCNTCTLRn
> @@ -799,7 +809,7 @@ static int etm4_parse_event_config(struct coresight_device *csdev,
> * order to correlate instructions executed on different CPUs
> * (CPU-wide trace scenarios).
> */
> - ret = etm4_config_timestamp_event(drvdata);
> + ret = etm4_config_timestamp_event(drvdata, attr);
>
> /*
> * No need to go further if timestamp intervals can't
>
> --
> 2.34.1
>
--
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK
Powered by blists - more mailing lists