Re: [PATCH v7 12/13] coresight: Allow setting the timestamp interval

[James Clark <james.clark@linaro.org>]

On 27/11/2025 3:48 pm, Mike Leach wrote:
> Hi James
> 
> On Wed, 26 Nov 2025 at 10:57, James Clark <james.clark@linaro.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@arm.com>
>> Tested-by: Leo Yan <leo.yan@arm.com>
>> Signed-off-by: James Clark <james.clark@linaro.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
> 

No, no change. This function never gets called if either timestamp or 
deprecated_timestamp = 0.

timestamp=0 and timestamp=1 still behave exactly the same as before. The 
only new change is timestamp values > 1 and that the bits used in the 
config are different.

>> +       /*
>> +        * 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
>>
> 
>