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.

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

MTL enters D3 power state using Zephyr Power Manager #7325

Merged
merged 5 commits into from
Jun 28, 2023
Merged
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
3 changes: 3 additions & 0 deletions app/boards/intel_adsp_ace15_mtpm.conf
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,15 @@ CONFIG_COMP_SRC_IPC4_FULL_MATRIX=y
CONFIG_PM=y
CONFIG_PM_DEVICE=y
CONFIG_PM_DEVICE_RUNTIME=y
CONFIG_PM_DEVICE_RUNTIME_EXCLUSIVE=n
CONFIG_PM_DEVICE_POWER_DOMAIN=y
CONFIG_PM_POLICY_CUSTOM=y

CONFIG_POWER_DOMAIN=y
CONFIG_POWER_DOMAIN_INTEL_ADSP=y

CONFIG_ADSP_IMR_CONTEXT_SAVE=y
aborisovich marked this conversation as resolved.
Show resolved Hide resolved

aborisovich marked this conversation as resolved.
Show resolved Hide resolved
# enable Zephyr drivers
CONFIG_ZEPHYR_NATIVE_DRIVERS=y
CONFIG_DAI=y
Expand Down
14 changes: 14 additions & 0 deletions src/include/sof/ipc/common.h
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,20 @@ int ipc_dai_data_config(struct dai_data *dd, struct comp_dev *dev);
*/
void ipc_boot_complete_msg(struct ipc_cmd_hdr *header, uint32_t data);

#if defined(CONFIG_PM_DEVICE) && defined(CONFIG_INTEL_ADSP_IPC)
/**
* @brief Send an IPC response to Host power transition request informing
* that power transition failed.
* @note Normally an reply to the Host IPC message is performed in the
* low level assembly code to make sure DSP completed all operations before
* power cut-off.
* However, when power transition fails for some reason, we should send the
* IPC response informing about the failure.
* This happens in abnormal circumstances since the response is send not during
* IPC task but during power transition logic in the Idle thread.
*/
void ipc_send_failed_power_transition_response(void);
#endif /* CONFIG_PM_DEVICE && CONFIG_INTEL_ADSP_IPC */
/**
* \brief Send a IPC notification that FW has hit
* a DSP notification.
Expand Down
83 changes: 79 additions & 4 deletions src/ipc/ipc-zephyr.c
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,15 @@
#include <sof/ipc/schedule.h>
aborisovich marked this conversation as resolved.
Show resolved Hide resolved
#include <sof/lib/mailbox.h>
#include <sof/lib/memory.h>
#if defined(CONFIG_PM_POLICY_CUSTOM)
#if defined(CONFIG_PM)
#include <sof/lib/cpu.h>
#include <zephyr/pm/device.h>
#include <zephyr/pm/state.h>
#include <zephyr/irq.h>
#include <zephyr/pm/policy.h>
#else
#include <sof/lib/pm_runtime.h>
#endif
#endif /* CONFIG_PM */
#include <sof/lib/uuid.h>
#include <rtos/wait.h>
#include <sof/list.h>
Expand All @@ -40,6 +44,8 @@
DECLARE_SOF_UUID("ipc-task", ipc_task_uuid, 0x8fa1d42f, 0xbc6f, 0x464b,
0x86, 0x7f, 0x54, 0x7a, 0xf0, 0x88, 0x34, 0xda);

LOG_MODULE_DECLARE(ipc, CONFIG_SOF_LOG_LEVEL);

/**
* @brief Private data for IPC.
*
Expand Down Expand Up @@ -79,6 +85,69 @@ static bool message_handler(const struct device *dev, void *arg, uint32_t data,
return false;
}

#ifdef CONFIG_PM_DEVICE
/**
* @brief IPC device suspend handler callback function.
* Checks whether device power state should be actually changed.
*
* @param dev IPC device.
* @param arg IPC struct pointer.
*/
static int ipc_device_suspend_handler(const struct device *dev, void *arg)
{
struct ipc *ipc = (struct ipc *)arg;

lyakh marked this conversation as resolved.
Show resolved Hide resolved
int ret = 0;

if (!(ipc->task_mask & IPC_TASK_POWERDOWN)) {
tr_err(&ipc_tr,
"ipc task mask not set to IPC_TASK_POWERDOWN. Current value: %u",
ipc->task_mask);
ret = -ENOMSG;
}

if (!ipc->pm_prepare_D3) {
tr_err(&ipc_tr, "power state D3 not requested");
ret = -EBADMSG;
}

if (!list_is_empty(&ipc->msg_list)) {
tr_err(&ipc_tr, "there are queued IPC messages to be sent");
ret = -EINPROGRESS;
}

if (ret != 0)
ipc_send_failed_power_transition_response();

return ret;
}

/**
* @brief IPC device resume handler callback function.
* Resets IPC control after context restore.
*
* @param dev IPC device.
* @param arg IPC struct pointer.
*/
static int ipc_device_resume_handler(const struct device *dev, void *arg)
{
struct ipc *ipc = (struct ipc *)arg;

ipc_set_drvdata(ipc, NULL);
ipc->task_mask = 0;
ipc->pm_prepare_D3 = false;

/* attach handlers */
intel_adsp_ipc_set_message_handler(INTEL_ADSP_IPC_HOST_DEV, message_handler, ipc);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not related to this patch, but realized this interface is not really well named, this should be used for all SOF platforms, not just Intel DSPs, so "intel_adsp_" prefix is not good here. But alas this is not added by this PR. FYI to @juimonen

Copy link
Member

Choose a reason for hiding this comment

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

Ah, so this needs to be generically named.


/* schedule task */
schedule_task_init_edf(&ipc->ipc_task, SOF_UUID(ipc_task_uuid),
&ipc_task_ops, ipc, 0, 0);

return 0;
}
#endif /* CONFIG_PM_DEVICE */

#if CONFIG_DEBUG_IPC_COUNTERS

static inline void increment_ipc_received_counter(void)
Expand Down Expand Up @@ -130,7 +199,7 @@ enum task_state ipc_platform_do_cmd(struct ipc *ipc)

if (ipc->task_mask & IPC_TASK_POWERDOWN ||
ipc_get()->pm_prepare_D3) {
#if defined(CONFIG_PM_POLICY_CUSTOM)
#if defined(CONFIG_PM)
/**
* @note For primary core this function
* will only force set lower power state
Expand All @@ -145,7 +214,7 @@ enum task_state ipc_platform_do_cmd(struct ipc *ipc)
* powered off and IPC sent.
*/
platform_pm_runtime_power_off();
#endif /* CONFIG_PM_POLICY_CUSTOM */
#endif /* CONFIG_PM */
}

return SOF_TASK_STATE_COMPLETED;
Expand Down Expand Up @@ -197,6 +266,12 @@ int platform_ipc_init(struct ipc *ipc)

/* attach handlers */
intel_adsp_ipc_set_message_handler(INTEL_ADSP_IPC_HOST_DEV, message_handler, ipc);
#ifdef CONFIG_PM
intel_adsp_ipc_set_suspend_handler(INTEL_ADSP_IPC_HOST_DEV,
ipc_device_suspend_handler, ipc);
intel_adsp_ipc_set_resume_handler(INTEL_ADSP_IPC_HOST_DEV,
ipc_device_resume_handler, ipc);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I guess these are no-ops on platforms where RAM context is not maintained in D3? I.e. it's ok to still set them.

#endif

return 0;
}
18 changes: 18 additions & 0 deletions src/ipc/ipc4/handler.c
Original file line number Diff line number Diff line change
Expand Up @@ -1068,6 +1068,24 @@ void ipc_boot_complete_msg(struct ipc_cmd_hdr *header, uint32_t data)
header->ext = 0;
}

#if defined(CONFIG_PM_DEVICE) && defined(CONFIG_INTEL_ADSP_IPC)
void ipc_send_failed_power_transition_response(void)
{
struct ipc4_message_request *request = ipc_from_hdr(&msg_data.msg_in);
struct ipc4_message_reply response;

response.primary.r.status = IPC4_POWER_TRANSITION_FAILED;
response.primary.r.rsp = SOF_IPC4_MESSAGE_DIR_MSG_REPLY;
response.primary.r.msg_tgt = request->primary.r.msg_tgt;
response.primary.r.type = request->primary.r.type;

msg_reply.header = response.primary.dat;
list_init(&msg_reply.list);

ipc_msg_send_direct(&msg_reply, NULL);
}
#endif /* defined(CONFIG_PM_DEVICE) && defined(CONFIG_INTEL_ADSP_IPC) */

void ipc_send_panic_notification(void)
{
msg_notify.header = SOF_IPC4_NOTIF_HEADER(SOF_IPC4_EXCEPTION_CAUGHT);
Expand Down
2 changes: 1 addition & 1 deletion src/platform/intel/ace/platform.c
aborisovich marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ int platform_boot_complete(uint32_t boot_message)
}

static struct pm_notifier pm_state_notifier = {
.state_entry = NULL,
.state_entry = cpu_notify_state_entry,
.state_exit = cpu_notify_state_exit,
};

Expand Down
2 changes: 2 additions & 0 deletions zephyr/include/sof/lib/cpu.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@

#include <zephyr/pm/pm.h>

void cpu_notify_state_entry(enum pm_state state);

void cpu_notify_state_exit(enum pm_state state);

#endif /* CONFIG_PM */
Expand Down
50 changes: 50 additions & 0 deletions zephyr/lib/cpu.c
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,14 @@
#include <sof/init.h>
#include <sof/lib/cpu.h>
#include <sof/lib/pm_runtime.h>
#include <ipc/topology.h>
#include <rtos/alloc.h>

/* Zephyr includes */
#include <version.h>
#include <zephyr/kernel.h>
#include <zephyr/device.h>
#include <zephyr/drivers/mm/mm_drv_intel_adsp_mtl_tlb.h>

#if CONFIG_MULTICORE && CONFIG_SMP

Expand Down Expand Up @@ -64,6 +68,42 @@ LOG_MODULE_DECLARE(zephyr, CONFIG_SOF_LOG_LEVEL);

extern struct tr_ctx zephyr_tr;

/* address where zephyr PM will save memory during D3 transition */
#ifdef CONFIG_ADSP_IMR_CONTEXT_SAVE
extern void *global_imr_ram_storage;
#endif

void cpu_notify_state_entry(enum pm_state state)
{
if (!cpu_is_primary(arch_proc_id()))
return;

if (state == PM_STATE_SOFT_OFF) {
#ifdef CONFIG_ADSP_IMR_CONTEXT_SAVE
size_t storage_buffer_size;

/* allocate IMR global_imr_ram_storage */
const struct device *tlb_dev = DEVICE_DT_GET(DT_NODELABEL(tlb));

__ASSERT_NO_MSG(tlb_dev);
const struct intel_adsp_tlb_api *tlb_api =
(struct intel_adsp_tlb_api *)tlb_dev->api;

/* get HPSRAM storage buffer size */
storage_buffer_size = tlb_api->get_storage_size();

/* add space for LPSRAM */
storage_buffer_size += LP_SRAM_SIZE;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Minor: it's not so obvious this is coming from Zephyr adsp_memory.h . Might be more readable to just directly use "DT_REG_SIZE(DT_NODELABEL(sram1)" as this a Zephyr-only file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, I would leave this matter to the author @marcinszkudlinski since it is his commit.
He is currently on vacation. Can we address this later? I will let him know.


/* allocate IMR buffer and store it in the global pointer */
global_imr_ram_storage = rmalloc(SOF_MEM_ZONE_SYS_RUNTIME,
0,
SOF_MEM_CAPS_L3,
storage_buffer_size);
#endif /* CONFIG_ADSP_IMR_CONTEXT_SAVE */
}
}

/* notifier called after every power state transition */
void cpu_notify_state_exit(enum pm_state state)
{
Expand All @@ -74,8 +114,18 @@ void cpu_notify_state_exit(enum pm_state state)
* state and is back in the Idle thread.
*/
atomic_set(&ready_flag, 1);
return;
}
#endif

#ifdef CONFIG_ADSP_IMR_CONTEXT_SAVE
/* free global_imr_ram_storage */
rfree(global_imr_ram_storage);
global_imr_ram_storage = NULL;

/* send FW Ready message */
platform_boot_complete(0);
#endif
}
}

Expand Down