Skip to content

PTL upstream #9185

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
mwasko merged 9 commits into from
Jun 24, 2024
Merged

PTL upstream #9185

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
06988c8
cmake/zephyr: change if condition to zephyr metod
pjdobrowolski Jun 7, 2024
1b3420c
platform: version: add hardware configuration version
pjdobrowolski Jun 14, 2024
67607b9
ptl: Add initial PTL configuration
jxstelter Nov 10, 2022
4248a11
intel_adsp: ptl: Enable chain-dma on PTL platform
makarukp Apr 17, 2023
215d595
intel_adsp: ptl: Enable LE HEAP on ptl
jxstelter May 24, 2023
18b4b5e
intel_adsp: ptl: Enable D3 flow on PTL
jxstelter Jul 13, 2023
0c6544d
ptl: Add FPGA overlay configuration
jxstelter May 25, 2023
d4be840
ptl: lnl: memory: expand HEAPMEM_SIZE
jxstelter Sep 22, 2023
f7502b8
tools: rimage: config: add PTL config
jxstelter Nov 30, 2023
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
76 changes: 76 additions & 0 deletions app/boards/intel_adsp_ace30_ptl.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
CONFIG_PANTHERLAKE=y
CONFIG_IPC_MAJOR_4=y
CONFIG_IPC4_BASE_FW_INTEL=y

CONFIG_COMP_SRC=y
CONFIG_MM_DRV=y
CONFIG_COMP_SRC_IPC4_FULL_MATRIX=y

# power settings
CONFIG_PM=y
CONFIG_PM_DEVICE=y
CONFIG_PM_DEVICE_RUNTIME=y
CONFIG_PM_DEVICE_POWER_DOMAIN=y
CONFIG_PM_POLICY_CUSTOM=y
CONFIG_ADSP_IMR_CONTEXT_SAVE=y

CONFIG_POWER_DOMAIN=y
CONFIG_POWER_DOMAIN_INTEL_ADSP=y

# enable Zephyr drivers
CONFIG_ZEPHYR_NATIVE_DRIVERS=y
CONFIG_DAI=y
CONFIG_DAI_INIT_PRIORITY=70
CONFIG_DAI_INTEL_DMIC=y
CONFIG_DAI_DMIC_HW_IOCLK=38400000
CONFIG_DAI_INTEL_DMIC_NHLT=y
CONFIG_DAI_DMIC_HAS_OWNERSHIP=n
CONFIG_DAI_DMIC_HAS_MULTIPLE_LINE_SYNC=y
CONFIG_DAI_INTEL_SSP=y
CONFIG_ZEPHYR_DP_SCHEDULER=y
CONFIG_DMA=y
CONFIG_DMA_INTEL_ADSP_GPDMA=n
CONFIG_DMA_DW_LLI_POOL_SIZE=50
CONFIG_INTEL_MODULES=y
CONFIG_LIBRARY_MANAGER=y
CONFIG_INTEL_ADSP_TIMER=y

CONFIG_HEAP_MEM_POOL_SIZE=8192
CONFIG_L3_HEAP=y
CONFIG_RIMAGE_SIGNING_SCHEMA="ptl"

CONFIG_FORMAT_CONVERT_HIFI3=n
CONFIG_PCM_CONVERTER_FORMAT_S16_C16_AND_S16_C32=y
CONFIG_PCM_CONVERTER_FORMAT_S16_C32_AND_S32_C32=y
CONFIG_PCM_CONVERTER_FORMAT_S16_C32_AND_S24_C32=y
CONFIG_PCM_CONVERTER_FORMAT_S24_C24_AND_S24_C32=y
CONFIG_PCM_CONVERTER_FORMAT_S24_C32_AND_S24_C24=y
CONFIG_PCM_CONVERTER_FORMAT_S16_C32_AND_S16_C32=y
CONFIG_LOG=y
CONFIG_LOG_MODE_DEFERRED=y
CONFIG_LOG_FUNC_NAME_PREFIX_ERR=y
CONFIG_LOG_FUNC_NAME_PREFIX_WRN=y
CONFIG_LOG_FUNC_NAME_PREFIX_INF=y
CONFIG_LOG_FUNC_NAME_PREFIX_DBG=y
CONFIG_COMP_VOLUME_WINDOWS_FADE=y
CONFIG_COMP_UP_DOWN_MIXER=y
CONFIG_COMP_CHAIN_DMA=y
CONFIG_SYS_CLOCK_TICKS_PER_SEC=12000

CONFIG_MM_DRV_INTEL_ADSP_TLB_REMAP_UNUSED_RAM=y

CONFIG_LOG_BACKEND_ADSP_MTRACE=y
CONFIG_SOF_LOG_LEVEL_INF=y

CONFIG_SOF_LOG_LEVEL_OFF=y
CONFIG_ZEPHYR_LOG=y
CONFIG_INTEL_ADSP_IPC=y

CONFIG_CLOCK_CONTROL_ADSP=y
CONFIG_CLOCK_CONTROL=y

CONFIG_COMP_KPB=y
CONFIG_COMP_ARIA=y

# Temporary disabled options
CONFIG_TRACE=n
66 changes: 66 additions & 0 deletions app/boards/intel_adsp_ace30_ptl_sim.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
CONFIG_PANTHERLAKE=y
CONFIG_IPC_MAJOR_4=y
CONFIG_IPC4_BASE_FW_INTEL=y

# turn off SOF drivers
CONFIG_COMP_SRC=y

CONFIG_COMP_SRC_IPC4_FULL_MATRIX=y

# power settings
CONFIG_PM=n
CONFIG_PM_POLICY_CUSTOM=y

# enable Zephyr drivers
CONFIG_ZEPHYR_NATIVE_DRIVERS=y
CONFIG_DAI=y
CONFIG_DAI_INIT_PRIORITY=70
CONFIG_DAI_INTEL_DMIC=y
CONFIG_DAI_DMIC_HW_IOCLK=19200000
CONFIG_DAI_INTEL_DMIC_NHLT=y
CONFIG_DAI_DMIC_HAS_OWNERSHIP=y
CONFIG_DAI_DMIC_HAS_MULTIPLE_LINE_SYNC=y
CONFIG_DAI_INTEL_SSP=n
CONFIG_DMA=y
CONFIG_DMA_INTEL_ADSP_GPDMA=n
CONFIG_DMA_DW_LLI_POOL_SIZE=50
CONFIG_INTEL_MODULES=n
CONFIG_LIBRARY_MANAGER=n
CONFIG_INTEL_ADSP_TIMER=y

CONFIG_HEAP_MEM_POOL_SIZE=8192
CONFIG_RIMAGE_SIGNING_SCHEMA="ptl"

CONFIG_FORMAT_CONVERT_HIFI3=n
CONFIG_PCM_CONVERTER_FORMAT_S16_C16_AND_S16_C32=y
CONFIG_PCM_CONVERTER_FORMAT_S16_C32_AND_S32_C32=y
CONFIG_PCM_CONVERTER_FORMAT_S16_C32_AND_S24_C32=y
CONFIG_PCM_CONVERTER_FORMAT_S24_C24_AND_S24_C32=y
CONFIG_PCM_CONVERTER_FORMAT_S24_C32_AND_S24_C24=y
CONFIG_PCM_CONVERTER_FORMAT_S16_C32_AND_S16_C32=y
CONFIG_LOG=n
CONFIG_LOG_MODE_DEFERRED=n
CONFIG_LOG_FUNC_NAME_PREFIX_INF=n
CONFIG_COMP_VOLUME_WINDOWS_FADE=y
CONFIG_COMP_UP_DOWN_MIXER=y
CONFIG_SYS_CLOCK_TICKS_PER_SEC=12000
CONFIG_SYS_CLOCK_HW_CYCLES_PER_SEC=19200000

CONFIG_LOG_BACKEND_ADSP_MTRACE=n
CONFIG_SOF_LOG_LEVEL_INF=n
CONFIG_SOF_LOG_LEVEL_OFF=y
CONFIG_ZEPHYR_LOG=n

CONFIG_INTEL_ADSP_IPC=y


# Temporary disabled options
CONFIG_TRACE=n
CONFIG_PM_DEVICE=y
CONFIG_PM_DEVICE_RUNTIME=n
CONFIG_PM_DEVICE_POWER_DOMAIN=n
CONFIG_COMP_KPB=n

CONFIG_CLOCK_CONTROL_ADSP=y
CONFIG_CLOCK_CONTROL=y
CONFIG_USERSPACE=y
3 changes: 3 additions & 0 deletions app/overlays/ptl/fpga_overlay.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
CONFIG_SYS_CLOCK_HW_CYCLES_PER_SEC=19200000
CONFIG_DAI_DMIC_HW_IOCLK=19200000

4 changes: 4 additions & 0 deletions app/sample.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ tests:
- intel_adsp/cavs25
- intel_adsp/ace15_mtpm
- intel_adsp/ace20_lnl
- intel_adsp/ace30_ptl
- intel_adsp/ace30_ptl_sim
- imx8qm_mek/mimx8qm6/adsp
- imx8qxp_mek/mimx8qx6/adsp
- imx8mp_evk/mimx8ml8/adsp
Expand All @@ -23,6 +25,8 @@ tests:
- intel_adsp/cavs25 # TGL
- intel_adsp/ace15_mtpm # MTL
- intel_adsp/ace20_lnl
- intel_adsp/ace30_ptl
- intel_adsp/ace30_ptl_sim
- imx8qm_mek/mimx8qm6/adsp
- imx8qxp_mek/mimx8qx6/adsp
- imx8mp_evk/mimx8ml8/adsp
Expand Down
12 changes: 12 additions & 0 deletions scripts/xtensa-build-zephyr.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,18 @@ class PlatformConfig:
"ace10_LX7HiFi4_2022_10",
ipc4 = True
),
"ptl" : PlatformConfig(
"intel", "intel_adsp/ace30_ptl",
f"RI-2022.10{xtensa_tools_version_postfix}",
"ace30_LX7HiFi4_PIF",
ipc4 = True
),
"ptl-sim" : PlatformConfig(
"intel", "intel_adsp/ace30_ptl_sim",
f"RI-2022.10{xtensa_tools_version_postfix}",
"ace30_LX7HiFi4_PIF",
ipc4 = True
),

# NXP platforms
"imx8" : PlatformConfig(
Expand Down
2 changes: 1 addition & 1 deletion src/audio/asrc/asrc.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
13, 0, 0, 0, 20480, 31871000, 384, 192, 0, 31871, 0,
14, 0, 0, 0, 20480, 34216000, 384, 256, 0, 34216, 0,
15, 0, 0, 0, 20480, 83448000, 1536, 1440, 0, 83448, 0]
#elif CONFIG_LUNARLAKE
#elif defined(CONFIG_LUNARLAKE) || defined(CONFIG_PANTHERLAKE)
mod_cfg = [0, 0, 0, 0, 20480, 4065600, 24, 22, 0, 0, 0,
1, 0, 0, 0, 20480, 5616000, 8, 25, 0, 0, 0,
2, 0, 0, 0, 20480, 7319200, 24, 27, 0, 0, 0,
Expand Down
2 changes: 1 addition & 1 deletion src/audio/copier/copier.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@
40, 0, 0, 0, 280, 6246000, 32, 32, 0, 6246, 0,
41, 0, 0, 0, 280, 5272000, 192, 384, 0, 5272, 0,
42, 0, 0, 0, 280, 5350000, 384, 192, 0, 5350, 0]
#elif CONFIG_LUNARLAKE
#elif defined(CONFIG_LUNARLAKE) || defined(CONFIG_PANTHERLAKE)
mod_cfg = [ 0, 0, 0, 0, 280, 640100, 45, 60, 0, 0, 0,
1, 0, 0, 0, 280, 1106300, 192, 192, 0, 0, 0,
2, 0, 0, 0, 280, 1573000, 45, 45, 0, 0, 0,
Expand Down
2 changes: 1 addition & 1 deletion src/audio/eq_iir/eq_iir.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
mod_cfg = [0, 0, 0, 0, 4096, 1000000, 128, 128, 0, 1000, 0,
0, 0, 0, 0, 4096, 20663000, 768, 768, 0, 20663, 0,
0, 0, 0, 0, 4096, 11357000, 384, 384, 0, 11357, 0]
#elif CONFIG_LUNARLAKE
#elif defined(CONFIG_LUNARLAKE) || defined(CONFIG_PANTHERLAKE)
mod_cfg = [0, 0, 0, 0, 4096, 1000000, 128, 128, 0, 0, 0]
#endif

Expand Down
4 changes: 2 additions & 2 deletions src/audio/mixin_mixout/mixin_mixout.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
3, 0, 0, 0, 296, 2928000, 512, 512, 0, 2928, 0,
4, 0, 0, 0, 296, 2572000, 128, 128, 0, 2572, 0,
5, 0, 0, 0, 296, 3760000, 1536, 1536, 0, 3760, 0]
#elif CONFIG_LUNARLAKE
#elif defined(CONFIG_LUNARLAKE) || defined(CONFIG_PANTHERLAKE)
mod_cfg = [ 0, 0, 0, 0, 296, 644000, 45, 60, 0, 0, 0,
1, 0, 0, 0, 296, 669900, 48, 64, 0, 0, 0,
2, 0, 0, 0, 296, 934000, 96, 128, 0, 0, 0,
Expand Down Expand Up @@ -70,7 +70,7 @@
3, 0, 0, 0, 520, 7631000, 512, 512, 0, 7631, 0,
4, 0, 0, 0, 520, 1953000, 128, 128, 0, 1953, 0,
5, 0, 0, 0, 520, 2301000, 1536, 1536, 0, 2301, 0]
#elif CONFIG_LUNARLAKE
#elif defined(CONFIG_LUNARLAKE) || defined(CONFIG_PANTHERLAKE)
mod_cfg = [0, 0, 0, 0, 520, 649600, 48, 64, 0, 0, 0,
1, 0, 0, 0, 520, 966300, 96, 128, 0, 0, 0,
2, 0, 0, 0, 520, 2101000, 48, 64, 0, 0, 0,
Expand Down
2 changes: 1 addition & 1 deletion src/audio/selector/selector.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
mod_cfg = [0, 0, 0, 0, 960, 488500, 16, 16, 0, 0, 0,
1, 0, 0, 0, 960, 964500, 16, 16, 0, 0, 0,
2, 0, 0, 0, 960, 2003000, 16, 16, 0, 0, 0]
#elif CONFIG_LUNARLAKE
#elif defined(CONFIG_LUNARLAKE) || defined(CONFIG_PANTHERLAKE)
mod_cfg = [0, 0, 0, 0, 216, 706000, 12, 16, 0, 0, 0,
1, 0, 0, 0, 216, 1271000, 8, 8, 0, 0, 0,
2, 0, 0, 0, 216, 1839000, 89, 118, 0, 0, 0,
Expand Down
2 changes: 1 addition & 1 deletion src/audio/src/src.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
24, 0, 0, 0, 12832, 27696000, 180, 256, 0, 27696, 0,
25, 0, 0, 0, 12832, 18368000, 256, 512, 0, 18368, 0,
26, 0, 0, 0, 12832, 15204000, 128, 256, 0, 15204, 0]
#elif CONFIG_LUNARLAKE
#elif defined(CONFIG_LUNARLAKE) || defined(CONFIG_PANTHERLAKE)
mod_cfg = [0, 0, 0, 0, 12832, 1365500, 0, 0, 0, 1365, 0,
1, 0, 0, 0, 12832, 2302300, 0, 0, 0, 2302, 0,
2, 0, 0, 0, 12832, 3218200, 0, 0, 0, 3218, 0,
Expand Down
2 changes: 1 addition & 1 deletion src/audio/up_down_mixer/up_down_mixer.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@
28, 0, 0, 0, 216, 5080000, 1536, 1152, 0, 5080, 0,
29, 0, 0, 0, 216, 3552000, 384, 1536, 0, 3552, 0,
30, 0, 0, 0, 216, 3728000, 768, 1152, 0, 3728, 0]
#elif CONFIG_LUNARLAKE
#elif defined(CONFIG_LUNARLAKE) || defined(CONFIG_PANTHERLAKE)
mod_cfg = [0, 0, 0, 0, 216, 706000, 12, 16, 0, 0, 0,
1, 0, 0, 0, 216, 1271000, 8, 8, 0, 0, 0,
2, 0, 0, 0, 216, 1839000, 89, 118, 0, 0, 0,
Expand Down
2 changes: 1 addition & 1 deletion src/audio/volume/gain.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
3, 0, 0, 0, 416, 8192000, 512, 512, 0, 8192, 0,
4, 0, 0, 0, 416, 10091000, 128, 128, 0, 10091, 0,
5, 0, 0, 0, 416, 5908000, 768, 768, 0, 5908, 0]
#elif CONFIG_LUNARLAKE
#elif defined(CONFIG_LUNARLAKE) || defined(CONFIG_PANTHERLAKE)
mod_cfg = [0, 0, 0, 0, 416, 914000, 48, 64, 0, 0, 0,
1, 0, 0, 0, 416, 1321600, 192, 256, 0, 0, 0,
2, 0, 0, 0, 416, 1786000, 192, 256, 0, 0, 0,
Expand Down
2 changes: 1 addition & 1 deletion src/audio/volume/peakvol.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
3, 0, 0, 0, 480, 12567000, 720, 720, 0, 12567, 0,
4, 0, 0, 0, 480, 7360000, 768, 768, 0, 7360, 0,
5, 0, 0, 0, 480, 12236000, 1536, 1536, 0, 12236, 0]
#elif CONFIG_LUNARLAKE
#elif defined(CONFIG_LUNARLAKE) || defined(CONFIG_PANTHERLAKE)
mod_cfg = [0, 0, 0, 0, 480, 1114000, 48, 64, 0, 0, 0,
1, 0, 0, 0, 480, 3321600, 192, 256, 0, 0, 0,
2, 0, 0, 0, 480, 3786000, 192, 256, 0, 0, 0,
Expand Down
8 changes: 4 additions & 4 deletions src/ipc/ipc4/dai.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ void dai_set_link_hda_config(uint16_t *link_config,
struct ipc_config_dai *common_config,
const void *spec_config)
{
#if defined(CONFIG_ACE_VERSION_2_0)
#if ACE_VERSION > ACE_VERSION_1_5
const struct ipc4_audio_format *out_fmt = common_config->out_fmt;
union hdalink_cfg link_cfg;

Expand Down Expand Up @@ -78,13 +78,13 @@ int dai_config_dma_channel(struct dai_data *dd, struct comp_dev *dev, const void
COMPILER_FALLTHROUGH;
case SOF_DAI_INTEL_DMIC:
channel = 0;
#if defined(CONFIG_ACE_VERSION_2_0)
#if ACE_VERSION > ACE_VERSION_1_5
if (dai->host_dma_config[0]->pre_allocated_by_host)
channel = dai->host_dma_config[0]->dma_channel_id;
#endif
break;
case SOF_DAI_INTEL_HDA:
#if defined(CONFIG_ACE_VERSION_2_0)
#if ACE_VERSION > ACE_VERSION_1_5
if (copier_cfg->gtw_cfg.node_id.f.dma_type == ipc4_alh_link_output_class ||
copier_cfg->gtw_cfg.node_id.f.dma_type == ipc4_alh_link_input_class) {
struct processing_module *mod = comp_mod(dev);
Expand All @@ -106,7 +106,7 @@ int dai_config_dma_channel(struct dai_data *dd, struct comp_dev *dev, const void
}
break;
}
#endif /* defined(CONFIG_ACE_VERSION_2_0) */
#endif /* ACE_VERSION > ACE_VERSION_1_5 */
channel = copier_cfg->gtw_cfg.node_id.f.v_index;
break;
case SOF_DAI_INTEL_ALH:
Expand Down
2 changes: 1 addition & 1 deletion src/ipc/ipc4/helper.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1076,7 +1076,7 @@ int ipc4_add_comp_dev(struct comp_dev *dev)

int ipc4_find_dma_config(struct ipc_config_dai *dai, uint8_t *data_buffer, uint32_t size)
{
#if defined(CONFIG_ACE_VERSION_2_0)
#if ACE_VERSION > ACE_VERSION_1_5
uint32_t *dma_config_id = GET_IPC_DMA_CONFIG_ID(data_buffer, size);

if (*dma_config_id != GTW_DMA_CONFIG_ID)
Expand Down
15 changes: 14 additions & 1 deletion src/platform/Kconfig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ config TIGERLAKE
help
Select if your target platform is Tigerlake-compatible


config METEORLAKE
bool "Build for Meteorlake"
select ACE
Expand All @@ -31,6 +30,13 @@ config LUNARLAKE
help
Select if your target platform is Lunarlake-compatible

config PANTHERLAKE
bool "Build for Pantherlake"
select ACE
select ACE_VERSION_3_0
help
Select if your target platform is Pantherlake-compatible

config LIBRARY
bool "Build Library"
help
Expand Down Expand Up @@ -249,6 +255,7 @@ endchoice
config MAX_CORE_COUNT
int
default 5 if LUNARLAKE
default 5 if PANTHERLAKE
Copy link
Contributor

@andyross andyross Jun 17, 2024

Choose a reason for hiding this comment

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

This is another spot where SOF is hiding a divergence from Zephyr. This should be using CONFIG_MP_MAX_NUM_CPUS, or if it has to be here for legacy builds there should be a BUILD_ASSERT somewhere to make sure they're identical.

Copy link
Member

@abonislawski abonislawski Jun 18, 2024

Choose a reason for hiding this comment

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

It was for compatibility in the past but right now it could be easily removed because we are already using CONFIG_MP_MAX_NUM_CPUS in most places.

In sof the most used core kconfig is CONFIG_CORE_COUNT which is based on zephyr

config CORE_COUNT
	int "Number of cores"
	default MP_MAX_NUM_CPUS if KERNEL_BIN_NAME = "zephyr"
	default MAX_CORE_COUNT

so this is a good thing to do but in a separate PR

default 4 if TIGERLAKE
default 3 if METEORLAKE
default 1
Expand Down Expand Up @@ -338,6 +345,12 @@ config ACE_VERSION_2_0
help
Select for ACE version 2.0

config ACE_VERSION_3_0
depends on ACE
bool
help
Select for ACE version 3.0
Copy link
Contributor

@andyross andyross Jun 20, 2024

Choose a reason for hiding this comment

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

Still whining that this is a 100% parallel and unconstrained recapitulation of platform selection that is already in the Zephyr tree. All this does is create the possibility of getting it wrong.

I get that this is "how it's always been done", but the time to fix that is now, when submitting a new platform. If PTL doesn't do it, then when will it happen?

At the very least (if the use of these older kconfigs is too hard to untangle) you should be deriving it from the existing constants and disallowing apps from setting it incorrectly:

  • No "help" text or docstring after the type, which prevents it from being overriden without a warning
  • default y if SOC_INTEL_ACE30_PTL sets it correctly always


config HP_MEMORY_BANKS
int "HP memory banks count"
depends on CAVS
Expand Down
15 changes: 14 additions & 1 deletion src/platform/intel/ace/include/ace/version.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,19 @@
#ifndef __ACE_VERSION_H__
#define __ACE_VERSION_H__

#define HW_CFG_VERSION 0
#define ACE_VERSION_1_5 0x10500 /* MTL */
#define ACE_VERSION_2_0 0x20000 /* LNL */
#define ACE_VERSION_3_0 0x30000 /* PTL */
Copy link
Contributor

@andyross andyross Jun 17, 2024

Choose a reason for hiding this comment

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

Can you check these names? TGL is cAVS 2.5, not ACE 1.5 (which is what Zephyr claims to be building for MTL, not 2.0!), etc.. Something looks mixed up.

Also: this kind of field-shifted versioning is a bad code smell. Everyone does it thinking that they'll be able to write "#if VERSION > whatever", but it never actually works that way over long term maintenance, because compatibility is hard, especially in hardware. And the hard constants end up in places they shouldn't, like binary formats, making changes to the scheme impossible in the future. Even if you don't want to use the Zephyr constants directly, strongly suggest junking these and using an enum. Everyone with a number-format version scheme ends up regretting the practice.

Copy link
Contributor

@andyross andyross Jun 20, 2024

Choose a reason for hiding this comment

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

Still really dislike introducing numbered versions for a system that doesn't guaranteed backward compatibility. Is a PTL device always compatible with a LNL one? Clearly no, right? So this is creating a promise you can't keep, even if the handful of spots in this PR work that way.

And to repeat: this is a new API, not a legacy one you're trying to accomodate.


/* ACE version defined by CONFIG_ACE_VER_*/
#if defined(CONFIG_ACE_VERSION_1_5)
#define ACE_VERSION ACE_VERSION_1_5
#elif defined(CONFIG_ACE_VERSION_2_0)
#define ACE_VERSION ACE_VERSION_2_0
#elif defined(CONFIG_ACE_VERSION_3_0)
#define ACE_VERSION ACE_VERSION_3_0
#endif
Copy link
Contributor

@andyross andyross Jun 17, 2024

Choose a reason for hiding this comment

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

Actually... what's the point of all the separately-defined version constants? Zephyr is already doing that for you with the board/soc{,_series,_family} hierarchy. At the very least this should be predicated on that. Doing otherwise opens up the possibility of building "pantherlake" against a build with CONFIG_SOC_SERIES_INTEL_CAVS_V25=y.

@ceolin @dcpleung are available for support if you need help figuring out the Zephyr hardware scheme, or if it's not doing something you need.


#define HW_CFG_VERSION ACE_VERSION

#endif /* __ACE_VERSION_H__ */
Loading