[PATCH i-g-t v3 1/3] lib/igt_perf: Use shift parameter for format

[Kamil Konieczny]

Hi Riana,
On 2025-02-20 at 16:55:54 +0530, Riana Tauro wrote:
> Replace start with shift and remove end parameter.

Maybe s/Replace/Rename/ ? Up to you.

> No functional changes
> 
> v2: fix documentation (Kamil)
> 
> Fixes: 8baa7d34b876 ("lib/igt_perf: Add utils to extract PMU event info")

You added here Fixes tag so please also fix function perf_event_config
first return should be 'return -errno' (this is improvement)
while second is wrong and should be 'return -EINVAL' (real fix)

Also describe all changes
s/No functional changes/...your descriptiom here...

> Signed-off-by: Riana Tauro <riana.tauro at intel.com>

with above fixes this is
Reviewed-by: Kamil Konieczny <kamil.konieczny at linux.intel.com>

> ---
>  lib/igt_perf.c       | 13 +++++++------
>  lib/igt_perf.h       |  2 +-
>  tests/intel/xe_pmu.c |  6 +++---
>  3 files changed, 11 insertions(+), 10 deletions(-)
> 
> diff --git a/lib/igt_perf.c b/lib/igt_perf.c
> index 2d0e699e3..d63371c92 100644
> --- a/lib/igt_perf.c
> +++ b/lib/igt_perf.c
> @@ -93,18 +93,19 @@ const char *xe_perf_device(int xe, char *buf, int buflen)
>  }
>  
>  /**
> - * perf_event_format: Returns the start/end positions of an event format param
> + * perf_event_format: Returns the shift for format param
>   * @device: PMU device
> - * @param: Parameter for which you need the format start/end bits
> + * @param: Parameter for which you need the format
> + * @shift: bit shift
>   *
>   * Returns: 0 on success or negative error code
>   */
> -int perf_event_format(const char *device, const char *param, uint32_t *start, uint32_t *end)
> +int perf_event_format(const char *device, const char *param, uint32_t *shift)
>  {
>  	char buf[NAME_MAX];
>  	ssize_t bytes;
> -	int ret;
> -	int fd;
> +	uint32_t end;
> +	int ret, fd;
>  
>  	snprintf(buf, sizeof(buf),
>  		 "/sys/bus/event_source/devices/%s/format/%s",
> @@ -120,7 +121,7 @@ int perf_event_format(const char *device, const char *param, uint32_t *start, ui
>  		return -EINVAL;
>  
>  	buf[bytes] = '\0';
> -	ret = sscanf(buf, "config:%u-%u", start, end);
> +	ret = sscanf(buf, "config:%u-%u", shift, &end);
>  	if (ret != 2)
>  		return -EINVAL;
>  
> diff --git a/lib/igt_perf.h b/lib/igt_perf.h
> index 69f7a3d74..58d7d47aa 100644
> --- a/lib/igt_perf.h
> +++ b/lib/igt_perf.h
> @@ -72,6 +72,6 @@ int perf_i915_open_group(int i915, uint64_t config, int group);
>  
>  int perf_xe_open(int xe, uint64_t config);
>  int perf_event_config(const char *device, const char *event, uint64_t *config);
> -int perf_event_format(const char *device, const char *param, uint32_t *start, uint32_t *end);
> +int perf_event_format(const char *device, const char *param, uint32_t *shift);
>  
>  #endif /* I915_PERF_H */
> diff --git a/tests/intel/xe_pmu.c b/tests/intel/xe_pmu.c
> index 62ed3a977..d408d0f99 100644
> --- a/tests/intel/xe_pmu.c
> +++ b/tests/intel/xe_pmu.c
> @@ -70,14 +70,14 @@ static uint64_t get_event_config(int xe, unsigned int gt, char *event)
>  	int ret;
>  	char xe_device[100];
>  	uint64_t pmu_config;
> -	u32 start, end;
> +	uint32_t shift;
>  
>  	xe_perf_device(xe, xe_device, sizeof(xe_device));
>  	ret = perf_event_config(xe_device, event, &pmu_config);
>  	igt_assert(ret >= 0);
> -	ret = perf_event_format(xe_device, "gt", &start, &end);
> +	ret = perf_event_format(xe_device, "gt", &shift);
>  	igt_assert(ret >= 0);
> -	pmu_config |= (uint64_t) gt << start;
> +	pmu_config |= (uint64_t)gt << shift;
>  
>  	return pmu_config;
>  }
> -- 
> 2.47.1
>