Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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: 1 addition & 1 deletion app/boards/intel_adsp_ace15_mtpm.conf
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ CONFIG_INTEL_ADSP_IPC=y
CONFIG_WATCHDOG=y
CONFIG_LL_WATCHDOG=y

CONFIG_MEMORY_WIN_2_SIZE=12288
CONFIG_MEMORY_WIN_2_SIZE=16384
Copy link
Collaborator

Choose a reason for hiding this comment

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

I wonder how we can make this scale better. Did @marcinszkudlinski you have ideas on this, I think you hit this with tracing backend as well.

I now pushed first PR for shell support zephyrproject-rtos/zephyr#72738 . I was contemplating on adding it to a new slot (and increase WIN_2 size), but:

  • shell won't be enable in all builds (just like tracing and probably this telemetry2 as well)
  • having to touch the CONFIG_MEMORY_WIN_2_SIZE requires touching multiple board specific files, so enabling the feature with a simpler overlay no longer works

No better solution yet. For shell, I now reuse the dw-slot for mtrace and force exclusivity between shell and mtrace.

Copy link
Member

Choose a reason for hiding this comment

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

Lets hard code today and we cant print it in teh tooling "I am using LNL so using hard codec value of X", in the future we can have a more dynamic approach.


# Temporary disabled options
CONFIG_TRACE=n
Expand Down
2 changes: 1 addition & 1 deletion app/boards/intel_adsp_ace20_lnl.conf
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ CONFIG_INTEL_ADSP_IPC=y
CONFIG_PROBE=y
CONFIG_PROBE_DMA_MAX=2

CONFIG_MEMORY_WIN_2_SIZE=12288
CONFIG_MEMORY_WIN_2_SIZE=16384


# Temporary disabled options
Expand Down
7 changes: 7 additions & 0 deletions app/prj.conf
Original file line number Diff line number Diff line change
Expand Up @@ -57,3 +57,10 @@ CONFIG_HEAP_MEM_POOL_SIZE=2048
CONFIG_LLEXT=y
CONFIG_LLEXT_STORAGE_WRITABLE=y
CONFIG_MODULES=y

#Thread info

CONFIG_SOF_TELEMETRY2_SLOT=y
CONFIG_SOF_TELEMETRY2_THREAD_INFO=y
CONFIG_THREAD_NAME=y
CONFIG_MEMORY_WIN_2_SIZE=16384
4 changes: 4 additions & 0 deletions src/debug/telemetry/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
# SPDX-License-Identifier: BSD-3-Clause

add_local_sources_ifdef(CONFIG_SOF_TELEMETRY sof telemetry.c)

add_local_sources_ifdef(CONFIG_SOF_TELEMETRY2_SLOT sof telemetry2_slot.c)

add_local_sources_ifdef(CONFIG_SOF_TELEMETRY2_THREAD_INFO sof telemetry2_thread_info.c)
43 changes: 43 additions & 0 deletions src/debug/telemetry/Kconfig
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,46 @@ config SOF_TELEMETRY
systick_info measurement which measures scheduler task performance and may
slightly affect overall performance.

config SOF_TELEMETRY2_SLOT
bool "Enable SOF telemetry2 slot in debug window"
Copy link
Member

Choose a reason for hiding this comment

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

what on earth is telemetry2?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think @lgirdwood 's original plan was to move the current telemetry node under a chunk in this new system, e.g. not to waste full 4k just for the current telemetry structure. That of course would need agreement from Windows side and some coordination, so for the time being I just decided to call it telemetry2.

Copy link
Member

Choose a reason for hiding this comment

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

The precisions are even less clear than the original, sorry. you're drowning me in details I didn't even know I didn't need :-)

help
TELEMETRY2 is a generic memory structure for passing real-time debug
information from the SOF system. It only provides the framework for
passing pieces of abstract data from DSP side to host side debugging
tools. The option enables TELEMETRY2 debug window slot for transferring
telemetry2 data.

if SOF_TELEMETRY2_SLOT

config SOF_TELEMETRY2_SLOT_NUMBER
int "Debug window slot where to put telemetry2 slot"
default 3
range 0 14
help
Which debug slot to reserve for telemetry2. Remember to map
the slot with MEMORY_WIN_2_SIZE in soc/xtensa/intel_adsp.

config SOF_TELEMETRY2_THREAD_INFO
bool "Enable thread info chunk in telemetry2 slot"
select INIT_STACKS
select THREAD_MONITOR
select THREAD_STACK_INFO
select THREAD_RUNTIME_STATS
help
This activates Zephyr thread info in telemetry2 slot. For
the thread names to be any more than hex numbers its suggested
to select THREAD_NAME=y too.

if SOF_TELEMETRY2_THREAD_INFO

config SOF_TELEMETRY2_THREAD_INFO_INTERVAL
int "Thread information collection interval in seconds"
default 2
range 1 10
help
Decides how often thread info runs and checks execution cycle
statistics and stack usage.

endif

endif
112 changes: 112 additions & 0 deletions src/debug/telemetry/telemetry2_slot.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
// SPDX-License-Identifier: BSD-3-Clause
//
// Copyright(c) 2024 Intel Corporation.

#include <zephyr/logging/log.h>
#include <zephyr/spinlock.h>
#include <adsp_debug_window.h>
#include <adsp_memory.h>
#include <sof/common.h>
#include "telemetry2_slot.h"

LOG_MODULE_REGISTER(telemetry2_slot);

static struct k_spinlock lock;

static
struct telemetry2_chunk_hdr *telemetry2_chunk_search(struct telemetry2_chunk_hdr *chunk,
int id)
{
uint8_t *p;

/* Loop through the chunks and look for 'id'. If found leave a
* pointer to it in 'chunk', if not leave 'chunk' pointing at
* the beginning of the free space.
*/
while (chunk->id != id && chunk->size != 0) {
p = (((uint8_t *) chunk) + chunk->size);

chunk = (struct telemetry2_chunk_hdr *) p;
}
return chunk;
}

/*
* Search, or if not found reserve, a chunk of specified size and id from
* telemetry2 slot. Each chuck must be aligned with a 64-byte cache line.
* A pointer to the chunk is returned.
*/
void *telemetry2_chunk_get(int id, size_t required_size)
{
const int slot = CONFIG_SOF_TELEMETRY2_SLOT_NUMBER;
struct telemetry2_payload_hdr *payload =
(struct telemetry2_payload_hdr *)(ADSP_DW->slots[slot]);
size_t hdr_size = ALIGN_UP(sizeof(*payload), CONFIG_DCACHE_LINE_SIZE);
struct telemetry2_chunk_hdr *chunk = (struct telemetry2_chunk_hdr *)
(((uint8_t *) payload) + hdr_size);
size_t size = ALIGN_UP(required_size, CONFIG_DCACHE_LINE_SIZE);
k_spinlock_key_t key;

if (ADSP_DW->descs[slot].type != ADSP_DW_SLOT_TELEMETRY2) {
if (ADSP_DW->descs[slot].type != 0)
LOG_WRN("Slot %d was not free: %u", slot, ADSP_DW->descs[slot].type);
LOG_INF("Initializing telemetry2 slot 0x%08x", ADSP_DW->descs[slot].type);
ADSP_DW->descs[slot].type = ADSP_DW_SLOT_TELEMETRY2;
payload->hdr_size = hdr_size;
payload->magic = TELEMETRY2_PAYLOAD_MAGIC;
payload->abi = TELEMETRY2_PAYLOAD_V0_0;
}

LOG_INF("Add id %u size %u (after alignment %u)", id, required_size, size);
chunk = telemetry2_chunk_search(chunk, id);
if (id == chunk->id)
goto match_found;

/* End of chunk list reached, but specified chunk not found. We
* need to reseeve one, so take the spin lock and check if more
* chunks were added since the previus search.
*/
key = k_spin_lock(&lock);
chunk = telemetry2_chunk_search(chunk, id);
/* Check if some other thread beat us to add the chunk we want */
if (id == chunk->id) {
k_spin_unlock(&lock, key);
goto match_found;
}

/* If the chunk was not found, reserve space for it after
* the last reserved chunk.
*/
if (((uint8_t *) chunk) + size >= ADSP_DW->slots[slot + 1]) {
LOG_ERR("No space for chunk %u of size %u in slot %u, offset %u",
id, size, slot, ((uint8_t *) chunk) - ADSP_DW->slots[slot]);
k_spin_unlock(&lock, key);
return NULL;
}

if (chunk->id != TELEMETRY2_CHUNK_ID_EMPTY)
LOG_WRN("Chunk of size %u has type %u, assuming empty",
chunk->size, chunk->id);

LOG_INF("Chunk %u reserved", id);
chunk->size = size;
chunk->id = id;
payload->total_size = size + ((uint8_t *) chunk) - ((uint8_t *) payload);

k_spin_unlock(&lock, key);

return chunk;

match_found:
/* If a matching chunk was found check that the size is enough
* for what was requested. If yes return the value, if not
* return NULL.
*/
if (required_size > chunk->size) {
LOG_ERR("Chunk %d size too small %u > %u",
id, required_size, chunk->size);
return NULL;
}
LOG_INF("Chunk %u found", id);
return chunk;
}
89 changes: 89 additions & 0 deletions src/debug/telemetry/telemetry2_slot.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
/* SPDX-License-Identifier: BSD-3-Clause
*
* Copyright(c) 2024 Intel Corporation.
*/

#ifndef __SOC_TELEMETRY2_SLOT_H__
#define __SOC_TELEMETRY2_SLOT_H__

/*
* TELEMETRY2 is a generic memory structure for passing real-time debug
* information from the SOF system. It only provides the framework for
* passing pieces of abstract data from DSP side to host side debugging
* tools.
*
* This example describes how TELEMETRY2 data is transferred from DSP
* to host side using debug memory window.
*
* TELEMETRY2 slot is taken from SRAM window divided into several
* chunks with simple header declared below.
*
* "type" describes the contents of the chunk
* "size" describes the size of the chunk in bytes
*
* After each chunk another chunk is expected to follow, until an
* empty chunk header is found. If chunk type is 0 and size 0 it
* indicates an empty header, and that the rest of the slot is unused.
* A new chunk, with the header, can be reserved stating at the end of
* previous chunk (which has been aligned to the end of cache line).
*
* Each chunk is aligned to start at 64-byte cache line boundary.
*
* For example:
* -------------------------------------------------- ---
* | magic = TELEMETRY2_PAYLOAD_MAGIC | |
* | hdr_size = 64 | |
* | total_size = 320 | |
* | abi = TELEMETRY2_PAYLOAD_V0_0 | 64 bytes
* | tstamp = <aligned with host epoch> | |
* | <padding> | |
* -------------------------------------------------- ---
* | type = ADSP_DW_TELEMETRY2_ID_THREAD_INFO | |
* | size = 256 | 256 bytes
Copy link
Member

Choose a reason for hiding this comment

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

can the size be different from a multiple of 64? IOW, can we have gaps due to alignment?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that is my intention. Thou, when I think about it I am not sure if this is a necessary requirement. I just had so many problems in the beginning with cache pecularities, I played it safe. But then again the debug window pointers are all uncached pointers, so maybe the system would survive two cores writing concurrently in the same cache line. I'll check.

* | chunk data | |
* | ... | |
* -------------------------------------------------- ---
* | type = ADSP_DW_TELEMETRY2_TYPE_EMPTY |
* | size = 0 |
* --------------------------------------------------
*
* The above depicts a telemetry2 slot with thread analyzer data
* of 256 bytes (including the header) in the first chunk and
* the rest of the slot being free.
*/

#include <stdint.h>

// HACK: This should be in zephyr/soc/intel/intel_adsp/common/include/adsp_debug_window.h
Copy link
Member

Choose a reason for hiding this comment

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

So if the payload is application specific we should be in SOF header, but RTOS specific in Zephyr header - if it s both then we cant keep in in application header.

#define ADSP_DW_SLOT_TELEMETRY2 0x4c455500

#define TELEMETRY2_PAYLOAD_MAGIC 0x1ED15EED

#define TELEMETRY2_PAYLOAD_V0_0 0

/* Telemetry2 payload header
*
* The payload header should be written in the beginning of the
* telemetry2 payload, before the chunks.
*/
struct telemetry2_payload_hdr {
uint32_t magic; // used to identify valid data
uint32_t hdr_size; // size of this header only in bytes
uint32_t total_size; // total size of the whole payload in bytes
Copy link
Member

Choose a reason for hiding this comment

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

including the header?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The payload header is only there in the beginning of the telemetry2 slot. I add some comments above. This is a recent addition from Liam's suggestion. I just changed it a bit so that the payload header can be treated as just another chunk.

uint32_t abi; // ABI version, can be used by tool to warn users if too old.
uint64_t tstamp; // aligned with host epoch.
Copy link
Member

Choose a reason for hiding this comment

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

how do we set the epoch?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, for the moment we don't. Its from Liam's comment. I actually do not think we should have it here. In the memory window each telemetry2 client is updating just their own chunk, and there can not be any universal timestamp for all of them. Each should have their own time-stamps indicating the time when the chunk was last updated. I would drop it. What do you say @lgirdwood ?

Copy link
Member

Choose a reason for hiding this comment

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

We should same epoch as logging.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, but in the case of debug window, the chunks may be updated independently. There is no universal timestamp that would be valid for the whole payload. If a timestamp is needed, each chunk should have its own stamp.

} __packed __aligned(CONFIG_DCACHE_LINE_SIZE);

/* Defined telemetry2 chunk types */
#define TELEMETRY2_CHUNK_ID_EMPTY 0
#define TELEMETRY2_ID_THREAD_INFO 1

/* Telemetry2 chunk header */
struct telemetry2_chunk_hdr {
uint32_t id; // Chunk ID
uint32_t size; // Size of telemetry chunk
} __packed;

void *telemetry2_chunk_get(int id, size_t size);

#endif /* __SOC_TELEMETRY2_SLOT_H__ */
Loading