Skip to content

IPC4 error handling #5276

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
lyakh wants to merge 2 commits into from
Closed

IPC4 error handling #5276

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d5d926f
ipc: add per-core context
lyakh Jan 27, 2022
6f254fb
ipc4: use standard POSIX error codes for function calls
lyakh Jan 27, 2022
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
1 change: 1 addition & 0 deletions src/arch/xtensa/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,7 @@ add_local_sources(sof
xtos/crt1-boards.S
xtos/_vectors.S
init.c
ipc.c
exc-dump.S
)

Expand Down
22 changes: 22 additions & 0 deletions src/arch/xtensa/ipc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// SPDX-License-Identifier: BSD-3-Clause
//
// Copyright(c) 2022 Intel Corporation. All rights reserved.
//
// Author: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>

/**
* @file
* @brief Xtensa IPC per-core data access
*/

#include <sof/ipc/common.h>
#include <sof/lib/alloc.h>
#include <sof/lib/cpu.h>
#include <xtos-structs.h>

struct ipc_core_ctx **arch_ipc_get(void)
{
struct core_context *ctx = (struct core_context *)cpu_read_threadptr();

return &ctx->ipc;
Copy link
Member

@lgirdwood lgirdwood Jan 27, 2022

Choose a reason for hiding this comment

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

Why do we need for xtos ?

Copy link
Collaborator Author

@lyakh lyakh Jan 28, 2022

Choose a reason for hiding this comment

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

It is currently possible to build IPC4 with XTOS, right? And I think that configuration is or was also used for the initial development / testing? So I thought I shouldn't break that functionality

Copy link
Member

@lgirdwood lgirdwood Jan 28, 2022

Choose a reason for hiding this comment

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

Lets use Zephyr for this work

Copy link
Collaborator Author

@lyakh lyakh Jan 31, 2022

Choose a reason for hiding this comment

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

@lgirdwood well, sure, I wouldn't mind that, but what exactly does that mean? Does that mean that it should only be possible to build SOF with IPC4 with Zephyr? So we should disable building IPC4 with XTOS? If we don't do that and keep the possibility to build SOF with IPC4, then how would error handling work there?

Copy link
Member

@lgirdwood lgirdwood Jan 31, 2022

Choose a reason for hiding this comment

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

@lyakh IPC4 with XTOS is not needed.

Copy link
Collaborator Author

@lyakh lyakh Apr 21, 2022

Choose a reason for hiding this comment

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

@lgirdwood I just learned from @ujfalusi that work is ongoing for IPC4 with XTOS

Copy link
Contributor

@ujfalusi ujfalusi Apr 21, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

@lyakh, to clarify: testing is done with both xtos and zephyr, but I personally build SOF with XTOS for my local development needs. The parts that I'm working with are way above anything xtos/zephyr and makes no difference for the end result.

Copy link
Collaborator Author

@lyakh lyakh Apr 22, 2022

Choose a reason for hiding this comment

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

@ujfalusi ok, got it, but IIUC it might happen that IPC4 will not even build with XTOS beginning from some point. In fact maybe we should add a Kconfig dependency for that.

}
3 changes: 3 additions & 0 deletions src/arch/xtensa/xtos/xtos-structs.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,12 +49,15 @@ struct xtos_core_data {
struct thread_data *thread_data_ptr;
};

struct ipc_core_ctx;

struct core_context {
struct thread_data td;
struct task *main_task;
struct schedulers *schedulers;
struct notify *notify;
struct idc *idc;
struct ipc_core_ctx *ipc;
};

#endif /* __XTOS_XTOS_STRUCTS_H__ */
7 changes: 7 additions & 0 deletions src/include/sof/ipc/common.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,13 @@ struct ipc {
#define ipc_get_drvdata(ipc) \
((ipc)->private)

struct ipc_core_ctx {
int error;
Copy link
Member

@lgirdwood lgirdwood Jan 27, 2022

Choose a reason for hiding this comment

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

this may not always be an error, but its more of a reply with context. maybe call it ipc4_reply.

};

struct ipc_core_ctx **arch_ipc_get(void);
int ipc_init_per_core(void);

extern struct task_ops ipc_task_ops;

/**
Expand Down
1 change: 0 additions & 1 deletion src/include/sof/ipc/topology.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ typedef uint32_t ipc_comp;

const struct comp_driver *ipc4_get_comp_drv(int module_id);
struct comp_dev *ipc4_get_comp_dev(uint32_t comp_id);
int ipc4_add_comp_dev(struct comp_dev *dev);
const struct comp_driver *ipc4_get_drv(uint8_t *uuid);
int ipc4_create_chain_dma(struct ipc *ipc, struct ipc4_chain_dma *cdma);
int ipc4_trigger_chain_dma(struct ipc *ipc, struct ipc4_chain_dma *cdma);
Expand Down
11 changes: 11 additions & 0 deletions src/init/init.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include <sof/debug/panic.h>
#include <sof/drivers/interrupt.h>
#include <sof/init.h>
#include <sof/ipc/common.h>
#include <sof/lib/cpu.h>
#include <sof/lib/memory.h>
#include <sof/lib/mm_heap.h>
Expand Down Expand Up @@ -172,6 +173,10 @@ int secondary_core_init(struct sof *sof)
if (err < 0)
return err;

err = ipc_init_per_core();
if (err < 0)
return err;

trace_point(TRACE_BOOT_PLATFORM);

#ifndef __ZEPHYR__
Expand All @@ -186,6 +191,8 @@ int secondary_core_init(struct sof *sof)

static int primary_core_init(int argc, char *argv[], struct sof *sof)
{
int err;

/* setup context */
sof->argc = argc;
sof->argv = argv;
Expand Down Expand Up @@ -219,6 +226,10 @@ static int primary_core_init(int argc, char *argv[], struct sof *sof)
if (platform_init(sof) < 0)
panic(SOF_IPC_PANIC_PLATFORM);

err = ipc_init_per_core();
if (err < 0)
return err;

trace_point(TRACE_BOOT_PLATFORM);

#if CONFIG_NO_SECONDARY_CORE_ROM
Expand Down
10 changes: 10 additions & 0 deletions src/ipc/ipc-common.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -297,3 +297,13 @@ struct task_ops ipc_task_ops = {
.complete = ipc_complete_task,
.get_deadline = ipc_task_deadline,
};

int ipc_init_per_core(void)
{
struct ipc_core_ctx **ctx = arch_ipc_get();

*ctx = rzalloc(SOF_MEM_ZONE_SYS, 0, SOF_MEM_CAPS_RAM,
sizeof(**ctx));

return *ctx ? 0 : -ENOMEM;
}
Loading