Skip to content

profiling task: modify task profiling with performance counter macro #6407

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
dbaluta merged 1 commit into from
Oct 14, 2022
Merged

profiling task: modify task profiling with performance counter macro #6407

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions src/include/sof/schedule/task.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
#include <stdbool.h>
#include <stdint.h>
#include <rtos/kernel.h>
#include <sof/lib/perf_cnt.h>


Copy link
Collaborator

@dbaluta dbaluta Oct 12, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You have now two new lines. Just keep one.

struct comp_dev;
struct sof;
Expand Down Expand Up @@ -72,6 +74,9 @@ struct task {
uint32_t cycles_max;
uint32_t cycles_cnt;
#endif
#if CONFIG_PERFORMANCE_COUNTERS
struct perf_cnt_data pcd;
#endif
};

static inline bool task_is_active(struct task *task)
Expand Down
28 changes: 8 additions & 20 deletions src/schedule/zephyr_ll.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
#include <sof/schedule/ll_schedule_domain.h>
#include <sof/schedule/schedule.h>
#include <sof/schedule/task.h>

#include <sof/lib/perf_cnt.h>
#include <zephyr/kernel.h>

LOG_MODULE_REGISTER(ll_schedule, CONFIG_SOF_LOG_LEVEL);
Expand Down Expand Up @@ -122,30 +122,18 @@ static void zephyr_ll_task_insert_after_unlocked(struct task *task, struct task

static inline enum task_state do_task_run(struct task *task)
{
uint32_t cycles0, cycles1, diff;
enum task_state state;

cycles0 = k_cycle_get_32();
#if CONFIG_PERFORMANCE_COUNTERS
perf_cnt_init(&task->pcd);
#endif

state = task_run(task);

cycles1 = k_cycle_get_32();
if (cycles1 > cycles0)
diff = cycles1 - cycles0;
else
diff = UINT32_MAX - cycles0 + cycles1;

task->cycles_sum += diff;
task->cycles_max = diff > task->cycles_max ? diff : task->cycles_max;

if (++task->cycles_cnt == 1 << CYCLES_WINDOW_SIZE) {
task->cycles_sum >>= CYCLES_WINDOW_SIZE;
tr_info(&ll_tr, "ll task %p %pU avg %u, max %u",
task, task->uid, task->cycles_sum, task->cycles_max);
task->cycles_sum = 0;
task->cycles_cnt = 0;
task->cycles_max = 0;
}
#if CONFIG_PERFORMANCE_COUNTERS
perf_cnt_stamp(&task->pcd, perf_trace_null, NULL);
task_perf_cnt_avg(&task->pcd, task_perf_avg_info, &ll_tr, task);
Copy link

@mmaka1 mmaka1 Oct 12, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

task_perf_... undefined?

Copy link
Contributor Author

@btian1 btian1 Oct 12, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

defined in #6353
I know, looks not good, the initial purpose want to put all perf_cnt changes in one PR.
does this acceptable? if not, I have to move the definition to this patch.

Copy link
Contributor Author

@btian1 btian1 Oct 12, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed to make each PR independent.

Copy link

@mmaka1 mmaka1 Oct 12, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should ..._avg be coupled with CONFIGU_PERFORMACE_COUNTERS_RUN_AVERAGE?

Copy link
Contributor Author

@btian1 btian1 Oct 12, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, in theory, it should, however, I even want to remove the CONFIG_PERFORMANCE_COUNTERS_RUN_AVERAGE, because since performance counter enabled, no need second switch, what do you think? if you agree, I can raise another patch to remove AVERAGE, I don't think it make too much sense.

I did not do this, because I don't want to break history, due to I am new, if you want a change, I can make it, :)

Copy link

@mmaka1 mmaka1 Oct 12, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I'd simplify and merge run_average into a single config option.

Copy link
Contributor Author

@btian1 btian1 Oct 12, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, I will submit another patch to do this after all are merged.

#endif

return state;
}
Expand Down
18 changes: 18 additions & 0 deletions xtos/include/sof/lib/perf_cnt.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,22 @@ struct perf_cnt_data {

/* perf measurement windows size 2^x */
#define PERF_CNT_CHECK_WINDOW_SIZE 10
#define task_perf_avg_info(pcd, task_p, class) \
tr_info(task_p, "perf_cycle task %p, %pU cpu avg %u peak %u",\
class, (class)->uid, \
(uint32_t)((pcd)->cpu_delta_sum), \
(uint32_t)((pcd)->cpu_delta_peak))
#define task_perf_cnt_avg(pcd, trace_m, arg, class) do { \
(pcd)->cpu_delta_sum += (pcd)->cpu_delta_last; \
if (++(pcd)->sample_cnt == 1 << PERF_CNT_CHECK_WINDOW_SIZE) { \
(pcd)->cpu_delta_sum >>= PERF_CNT_CHECK_WINDOW_SIZE; \
trace_m(pcd, arg, class); \
(pcd)->cpu_delta_sum = 0; \
(pcd)->sample_cnt = 0; \
(pcd)->plat_delta_peak = 0; \
(pcd)->cpu_delta_peak = 0; \
} \
} while (0)

/** \brief Accumulates cpu timer delta samples calculated by perf_cnt_stamp().
*
Expand All @@ -100,6 +116,8 @@ struct perf_cnt_data {

#else
#define perf_cnt_average(pcd, trace_m, arg)
#define task_perf_cnt_avg(pcd, trace_m, arg, class)
#define task_perf_avg_info(pcd, task_p, class)
#endif /* CONFIG_PERFORMANCE_COUNTERS_RUN_AVERAGE */

/** \brief Reads the timers and computes delta to the previous readings.
Expand Down