Re: [PATCH v5 09/22] cpufreq: amd: add trace for amd-pstate module

["Rafael J. Wysocki" <rafael@kernel.org>]

On Tue, Nov 30, 2021 at 1:38 PM Huang Rui <ray.huang@amd.com> wrote:
>
> Add trace event to monitor the performance value changes which is
> controlled by cpu governors.

This would need an ACK from Steve.

>
> Signed-off-by: Huang Rui <ray.huang@amd.com>
> ---
>  drivers/cpufreq/Makefile           |  6 ++-
>  drivers/cpufreq/amd-pstate-trace.c |  2 +
>  drivers/cpufreq/amd-pstate-trace.h | 77 ++++++++++++++++++++++++++++++

Why are these two extra files necessary?

>  drivers/cpufreq/amd-pstate.c       |  4 ++
>  4 files changed, 88 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/cpufreq/amd-pstate-trace.c
>  create mode 100644 drivers/cpufreq/amd-pstate-trace.h
>
> diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile
> index c8d307010922..285de70af877 100644
> --- a/drivers/cpufreq/Makefile
> +++ b/drivers/cpufreq/Makefile
> @@ -17,6 +17,10 @@ obj-$(CONFIG_CPU_FREQ_GOV_ATTR_SET)  += cpufreq_governor_attr_set.o
>  obj-$(CONFIG_CPUFREQ_DT)               += cpufreq-dt.o
>  obj-$(CONFIG_CPUFREQ_DT_PLATDEV)       += cpufreq-dt-platdev.o
>
> +# Traces
> +CFLAGS_amd-pstate-trace.o               := -I$(src)
> +amd_pstate-y                           := amd-pstate.o amd-pstate-trace.o
> +
>  ##################################################################################
>  # x86 drivers.
>  # Link order matters. K8 is preferred to ACPI because of firmware bugs in early
> @@ -25,7 +29,7 @@ obj-$(CONFIG_CPUFREQ_DT_PLATDEV)      += cpufreq-dt-platdev.o
>  # speedstep-* is preferred over p4-clockmod.
>
>  obj-$(CONFIG_X86_ACPI_CPUFREQ)         += acpi-cpufreq.o
> -obj-$(CONFIG_X86_AMD_PSTATE)           += amd-pstate.o
> +obj-$(CONFIG_X86_AMD_PSTATE)           += amd_pstate.o
>  obj-$(CONFIG_X86_POWERNOW_K8)          += powernow-k8.o
>  obj-$(CONFIG_X86_PCC_CPUFREQ)          += pcc-cpufreq.o
>  obj-$(CONFIG_X86_POWERNOW_K6)          += powernow-k6.o
> diff --git a/drivers/cpufreq/amd-pstate-trace.c b/drivers/cpufreq/amd-pstate-trace.c
> new file mode 100644
> index 000000000000..891b696dcd69
> --- /dev/null
> +++ b/drivers/cpufreq/amd-pstate-trace.c
> @@ -0,0 +1,2 @@
> +#define CREATE_TRACE_POINTS
> +#include "amd-pstate-trace.h"
> diff --git a/drivers/cpufreq/amd-pstate-trace.h b/drivers/cpufreq/amd-pstate-trace.h
> new file mode 100644
> index 000000000000..647505957d4f
> --- /dev/null
> +++ b/drivers/cpufreq/amd-pstate-trace.h
> @@ -0,0 +1,77 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * amd-pstate-trace.h - AMD Processor P-state Frequency Driver Tracer
> + *
> + * Copyright (C) 2021 Advanced Micro Devices, Inc. All Rights Reserved.
> + *
> + * Author: Huang Rui <ray.huang@amd.com>
> + */
> +
> +#if !defined(_AMD_PSTATE_TRACE_H) || defined(TRACE_HEADER_MULTI_READ)
> +#define _AMD_PSTATE_TRACE_H
> +
> +#include <linux/cpufreq.h>
> +#include <linux/tracepoint.h>
> +#include <linux/trace_events.h>
> +
> +#undef TRACE_SYSTEM
> +#define TRACE_SYSTEM amd_cpu
> +
> +#undef TRACE_INCLUDE_FILE
> +#define TRACE_INCLUDE_FILE amd-pstate-trace
> +
> +#define TPS(x)  tracepoint_string(x)
> +
> +TRACE_EVENT(amd_pstate_perf,

Could this be added to include/trace/events/power.h ?

> +
> +       TP_PROTO(unsigned long min_perf,
> +                unsigned long target_perf,
> +                unsigned long capacity,
> +                unsigned int cpu_id,
> +                bool changed,
> +                bool fast_switch
> +                ),
> +
> +       TP_ARGS(min_perf,
> +               target_perf,
> +               capacity,
> +               cpu_id,
> +               changed,
> +               fast_switch
> +               ),
> +
> +       TP_STRUCT__entry(
> +               __field(unsigned long, min_perf)
> +               __field(unsigned long, target_perf)
> +               __field(unsigned long, capacity)
> +               __field(unsigned int, cpu_id)
> +               __field(bool, changed)
> +               __field(bool, fast_switch)
> +               ),
> +
> +       TP_fast_assign(
> +               __entry->min_perf = min_perf;
> +               __entry->target_perf = target_perf;
> +               __entry->capacity = capacity;
> +               __entry->cpu_id = cpu_id;
> +               __entry->changed = changed;
> +               __entry->fast_switch = fast_switch;
> +               ),
> +
> +       TP_printk("amd_min_perf=%lu amd_des_perf=%lu amd_max_perf=%lu cpu_id=%u changed=%s fast_switch=%s",
> +                 (unsigned long)__entry->min_perf,
> +                 (unsigned long)__entry->target_perf,
> +                 (unsigned long)__entry->capacity,
> +                 (unsigned int)__entry->cpu_id,
> +                 (__entry->changed) ? "true" : "false",
> +                 (__entry->fast_switch) ? "true" : "false"
> +                )
> +);
> +
> +#endif /* _AMD_PSTATE_TRACE_H */
> +
> +/* This part must be outside protection */
> +#undef TRACE_INCLUDE_PATH
> +#define TRACE_INCLUDE_PATH .
> +
> +#include <trace/define_trace.h>
> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> index 68991c450fd5..72a4e2258fe7 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -31,6 +31,7 @@
>  #include <asm/processor.h>
>  #include <asm/cpufeature.h>
>  #include <asm/cpu_device_id.h>
> +#include "amd-pstate-trace.h"
>
>  #define AMD_PSTATE_TRANSITION_LATENCY  0x20000
>  #define AMD_PSTATE_TRANSITION_DELAY    500
> @@ -189,6 +190,9 @@ static void amd_pstate_update(struct amd_cpudata *cpudata, u32 min_perf,
>         value &= ~REQ_MAX_PERF(~0L);
>         value |= REQ_MAX_PERF(max_perf);
>
> +       trace_amd_pstate_perf(min_perf, des_perf, max_perf,
> +                             cpudata->cpu, (value != prev), fast_switch);
> +
>         if (value == prev)
>                 return;
>
> --
> 2.25.1
>