Skip to content
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

audio: make clock control optional in SOF Zephyr builds (plus remove clk platform code for TGL as example) #9670

Merged
merged 7 commits into from
Nov 26, 2024
Merged

audio: make clock control optional in SOF Zephyr builds (plus remove clk platform code for TGL as example) #9670

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
cb4129d
audio: explicitly include rtos/clk.h for CLK_ macros
kv2019i Nov 20, 2024
0046fac
zephyr: do not use clock_get_freq() in platform_dai_timestamp()
kv2019i Nov 20, 2024
a5fc213
lib_manager: make CPU clock control calls optional
kv2019i Nov 21, 2024
a51bf68
platform: intel: ace: make CPU clock control calls optional
kv2019i Nov 21, 2024
3fef3e3
audio: make clock control optional in SOF Zephyr builds
kv2019i Nov 20, 2024
6b5e6c3
audio: base_fw: do not include platform headers directly
kv2019i Nov 20, 2024
f6d7d4c
platform: tigerlake: remove use of SOF clock interface
kv2019i Nov 25, 2024
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
12 changes: 11 additions & 1 deletion src/audio/base_fw.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@
#include <sof/lib/cpu.h>
#include <sof/platform.h>
#include <sof/lib_manager.h>
#include <rtos/clk.h>
#include <rtos/init.h>
#include <platform/lib/clk.h>
#include <sof/audio/module_adapter/module/generic.h>
#include <sof/schedule/dp_schedule.h>
#include <sof/schedule/ll_schedule.h>
Expand Down Expand Up @@ -57,13 +57,15 @@ static int basefw_config(uint32_t *data_offset, char *data)
tuple = tlv_next(tuple);
tlv_value_uint32_set(tuple, IPC4_MEMORY_RECLAIMED_FW_CFG, 1);

#ifndef CONFIG_SOF_ZEPHYR_NO_SOF_CLOCK
tuple = tlv_next(tuple);
tlv_value_uint32_set(tuple, IPC4_FAST_CLOCK_FREQ_HZ_FW_CFG, CLK_MAX_CPU_HZ);

tuple = tlv_next(tuple);
tlv_value_uint32_set(tuple,
IPC4_SLOW_CLOCK_FREQ_HZ_FW_CFG,
clock_get_freq(CPU_LOWEST_FREQ_IDX));
#endif

tuple = tlv_next(tuple);
tlv_value_uint32_set(tuple, IPC4_DL_MAILBOX_BYTES_FW_CFG, MAILBOX_HOSTBOX_SIZE);
Expand Down Expand Up @@ -219,17 +221,21 @@ static int basefw_register_kcps(bool first_block,
if (!(first_block && last_block))
return IPC4_ERROR_INVALID_PARAM;

#if CONFIG_KCPS_DYNAMIC_CLOCK_CONTROL
/* value of kcps to request on core 0. Can be negative */
if (core_kcps_adjust(0, *(int32_t *)data))
return IPC4_ERROR_INVALID_PARAM;
#endif

return IPC4_SUCCESS;
}

static int basefw_kcps_allocation_request(struct ipc4_resource_kcps *request)
{
#if CONFIG_KCPS_DYNAMIC_CLOCK_CONTROL
if (core_kcps_adjust(request->core_id, request->kcps))
return IPC4_ERROR_INVALID_PARAM;
#endif

return IPC4_SUCCESS;
}
Expand Down Expand Up @@ -258,6 +264,7 @@ static int basefw_resource_allocation_request(bool first_block,

static int basefw_power_state_info_get(uint32_t *data_offset, char *data)
{
#if CONFIG_KCPS_DYNAMIC_CLOCK_CONTROL
struct sof_tlv *tuple = (struct sof_tlv *)data;
uint32_t core_kcps[CONFIG_CORE_COUNT] = {0};
int core_id;
Expand All @@ -274,6 +281,9 @@ static int basefw_power_state_info_get(uint32_t *data_offset, char *data)
tuple = tlv_next(tuple);
*data_offset = (int)((char *)tuple - data);
return IPC4_SUCCESS;
#else
return IPC4_UNAVAILABLE;
#endif
}

static int basefw_libraries_info_get(uint32_t *data_offset, char *data)
Expand Down
2 changes: 1 addition & 1 deletion src/audio/pipeline/pipeline-graph.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,7 @@ int pipeline_complete(struct pipeline *p, struct comp_dev *source,
.comp_data = &data,
};

#if !UNIT_TEST && !CONFIG_LIBRARY
#if !UNIT_TEST && !CONFIG_LIBRARY && CONFIG_KCPS_DYNAMIC_CLOCK_CONTROL
int __maybe_unused freq = clock_get_freq(cpu_get_id());
#else
int __maybe_unused freq = 0;
Expand Down
1 change: 1 addition & 0 deletions src/audio/pipeline/pipeline-stream.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include <rtos/interrupt.h>
#include <rtos/spinlock.h>
#include <rtos/string.h>
#include <rtos/clk.h>
#include <ipc/stream.h>
#include <ipc/topology.h>
#include <ipc4/module.h>
Expand Down
7 changes: 7 additions & 0 deletions src/library_manager/lib_manager.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include <sof/compiler_attributes.h>
#include <sof/ipc/topology.h>

#include <rtos/clk.h>
#include <rtos/sof.h>
#include <rtos/spinlock.h>
#include <sof/lib/cpu-clk-manager.h>
Expand Down Expand Up @@ -962,13 +963,15 @@ static int lib_manager_setup(uint32_t dma_id)

dma_block_cfg.dest_address = dma_ext->dma_addr;

#if CONFIG_KCPS_DYNAMIC_CLOCK_CONTROL
/*
* make sure that the DSP is running full speed for the duration of
* library loading
*/
ret = core_kcps_adjust(cpu_get_id(), CLK_MAX_CPU_HZ / 1000);
if (ret < 0)
goto err_dma_buffer;
#endif
Copy link
Collaborator

Choose a reason for hiding this comment

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

maybe a dummy inline core_kcps_adjust() implementation instead?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@lyakh Tried this approach, but it gets a bit ugly as CLK_MAX_CPU_HZ is not defined either. Or we want to drop the requirement to define such values on SOF side.


ret = dma_config(dma_ext->chan->dma->z_dev, dma_ext->chan->index, &config);
if (ret < 0)
Expand All @@ -983,7 +986,9 @@ static int lib_manager_setup(uint32_t dma_id)
return 0;

err_dma:
#if CONFIG_KCPS_DYNAMIC_CLOCK_CONTROL
core_kcps_adjust(cpu_get_id(), -(CLK_MAX_CPU_HZ / 1000));
#endif

err_dma_buffer:
lib_manager_dma_deinit(dma_ext, dma_id);
Expand Down Expand Up @@ -1061,7 +1066,9 @@ int lib_manager_load_library(uint32_t dma_id, uint32_t lib_id, uint32_t type)
rfree((__sparse_force void *)man_tmp_buffer);

cleanup:
#if CONFIG_KCPS_DYNAMIC_CLOCK_CONTROL
core_kcps_adjust(cpu_get_id(), -(CLK_MAX_CPU_HZ / 1000));
#endif
rfree((void *)dma_ext->dma_addr);
lib_manager_dma_deinit(dma_ext, dma_id);
rfree(dma_ext);
Expand Down
1 change: 1 addition & 0 deletions src/platform/Kconfig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,7 @@ config CAVS
default n
select INTEL
select INTEL_MN
select SOF_ZEPHYR_NO_SOF_CLOCK

config CAVS_VERSION_2_5
depends on CAVS
Expand Down
7 changes: 3 additions & 4 deletions src/platform/intel/ace/platform.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,15 +90,14 @@ int platform_init(struct sof *sof)

trace_point(TRACE_BOOT_PLATFORM_CLOCK);
platform_clock_init(sof);
kcps_budget_init();

#if CONFIG_KCPS_DYNAMIC_CLOCK_CONTROL
kcps_budget_init();

ret = core_kcps_adjust(cpu_get_id(), PRIMARY_CORE_BASE_CPS_USAGE);
#else
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@abonislawski @serhiy-katsyuba-intel Can you double-check this? It seems unnecessary to set the DSP frequency from SOF side if there is no dynamic control (the Zephyr platform code should choose the default speed).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@abonislawski any comment?

ret = core_kcps_adjust(cpu_get_id(), CLK_MAX_CPU_HZ / 1000);
#endif
if (ret < 0)
return ret;
Copy link
Collaborator

Choose a reason for hiding this comment

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

please, move this check under #ifdef and remove ret initialisation

Copy link
Collaborator Author

@kv2019i kv2019i Nov 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Done in today's PR revision.
UPDATE: missed the ret initialization, will update.

#endif

trace_point(TRACE_BOOT_PLATFORM_SCHED);
scheduler_init_edf();
Expand Down
54 changes: 0 additions & 54 deletions src/platform/intel/cavs/include/cavs/lib/clk.h
View file
Open in desktop

This file was deleted.

6 changes: 2 additions & 4 deletions src/platform/intel/cavs/platform.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,14 +116,12 @@ int platform_init(struct sof *sof)
{
int ret;

trace_point(TRACE_BOOT_PLATFORM_CLOCK);
platform_clock_init(sof);

trace_point(TRACE_BOOT_PLATFORM_SCHED);
scheduler_init_edf();

/* init low latency timer domain and scheduler */
sof->platform_timer_domain = timer_domain_init(sof->platform_timer, PLATFORM_DEFAULT_CLOCK);
/* clk is ignored on Zephyr so pass 0 */
sof->platform_timer_domain = timer_domain_init(sof->platform_timer, 0);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The "clk" parameter has no use on Zephyer, so might as well pass a 0.

Copy link
Collaborator

Choose a reason for hiding this comment

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

deserves a comment?

Copy link
Collaborator Author

@kv2019i kv2019i Nov 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Done in today's PR revision.
UPDATE: comment really udpated in the second PR revision of today

scheduler_init_ll(sof->platform_timer_domain);

/* init the system agent */
Expand Down
41 changes: 0 additions & 41 deletions src/platform/tigerlake/include/platform/lib/clk.h
View file
Open in desktop

This file was deleted.

11 changes: 2 additions & 9 deletions src/platform/tigerlake/include/platform/platform.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,18 +19,11 @@
#include <stddef.h>
#include <stdint.h>

#include <cavs/version.h>

struct ll_schedule_domain;
struct timer;

/*! \def PLATFORM_DEFAULT_CLOCK
* \brief clock source for audio pipeline
*
* There are two types of clock: cpu clock which is a internal clock in
* xtensa core, and ssp clock which is provided by external HW IP.
* The choice depends on HW features on different platform
*/
#define PLATFORM_DEFAULT_CLOCK CLK_SSP

/* Host page size */
#define HOST_PAGE_SIZE 4096

Expand Down
91 changes: 0 additions & 91 deletions src/platform/tigerlake/lib/clk.c
View file
Open in desktop

This file was deleted.

Loading
Loading