Skip to content

sof performance build enabling #6322

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

Closed
btian1 wants to merge 1 commit into from
Closed

sof performance build enabling #6322

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
2 changes: 2 additions & 0 deletions app/perf_overlay.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
CONFIG_PERFORMANCE_COUNTERS=y
Copy link
Collaborator

@ranj063 ranj063 Sep 23, 2022

Choose a reason for hiding this comment

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

@btian1 Can you split this patch into 2 separate patches with the first one to introduce the macros and the second one for enabling perf build?

Copy link
Collaborator

@ranj063 ranj063 Sep 23, 2022

Choose a reason for hiding this comment

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

Also, Can you please fix your name in the sign-off?

Copy link
Contributor Author

@btian1 btian1 Sep 26, 2022

Choose a reason for hiding this comment

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

let me check, I already tried to fix sign off.
ok, seems change too much in one patch is not a good way , let me split based on you, marc, Liam's requirement

Copy link
Contributor

@andrula-song andrula-song Sep 27, 2022

Choose a reason for hiding this comment

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

would it be better to keep the value NO?

Copy link
Collaborator

@marc-hb marc-hb Sep 27, 2022

Choose a reason for hiding this comment

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

@andrula-song this overlay is not used by default.

Copy link
Collaborator

@kv2019i kv2019i Oct 11, 2022

Choose a reason for hiding this comment

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

@btian1 The signed-off is still wrong, you should have proper full name there.
Also, I recommend @ranj063 's idea of splitting to two patches. One to move the task perf tracking to new interface and a separate patch to add the performance build.

Copy link
Collaborator

@singalsu singalsu Oct 10, 2022

Choose a reason for hiding this comment

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

Nit, start commit message sentences with capital letters. Doesn't checkpatch complain about this.

CONFIG_PERFORMANCE_COUNTERS_RUN_AVERAGE=y
4 changes: 3 additions & 1 deletion src/include/sof/audio/component.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,9 @@ enum {
(uint32_t)((pcd)->cpu_delta_peak))

#define comp_perf_avg_info(pcd, comp_p) \
comp_info(comp_p, "perf comp_copy cpu avg %u (current peak %u)",\
comp_info(comp_p, "perf comp_copy samples %u period %u cpu avg %u peak %u",\
(uint32_t)((comp_p)->frames), \
(uint32_t)((comp_p)->period), \
Copy link
Collaborator

@marc-hb marc-hb Oct 10, 2022

Choose a reason for hiding this comment

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

frames and period are defined as uint32_t in src/include/sof/audio/component.h. Why are they cast to uint32_t? Casts hide bugs from the compiler, we want fewer casts not more.

Copy link
Collaborator

@marc-hb marc-hb Oct 10, 2022

Choose a reason for hiding this comment

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

BTW renaming comp_p to comp_dev_p would save a lot of time to people new to this code (which involves an unreasonable number of macro indirections for "functional programming in C")

(uint32_t)((pcd)->cpu_delta_sum), \
(uint32_t)((pcd)->cpu_delta_peak))

Expand Down
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>


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);
#endif

return state;
}
Expand Down