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

Jump to bottom

pm: device_runtime: Async improvements #66246

Merged
merged 5 commits into from Dec 12, 2023
Merged

pm: device_runtime: Async improvements #66246

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
164fcc4
pm: device_runtime: Avoid unnecessary resume/suspend
ceolin Dec 6, 2023
7019506
pm: device_runtime: Add delay to async put
ceolin Dec 5, 2023
8cb596e
tests: pm: device_runtime: Checks async put with delay
ceolin Dec 6, 2023
bef11ec
tests: pm: device_runtime_api: Check async behavior
ceolin Dec 6, 2023
5991f85
soc: stm32f4: Fix PM dependency
ceolin Dec 7, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion doc/services/pm/device_runtime.rst
View file
Open in desktop
Expand Up @@ -224,5 +224,5 @@ asynchronous API:
...

/* "put" device (decreases usage count, schedule suspend if no more users) */
return pm_device_runtime_put_async(dev);
return pm_device_runtime_put_async(dev, K_NO_WAIT);
}
7 changes: 5 additions & 2 deletions include/zephyr/pm/device_runtime.h
View file
Open in desktop
Expand Up @@ -131,6 +131,7 @@ int pm_device_runtime_put(const struct device *dev);
* @funcprops \pre_kernel_ok, \async, \isr_ok
*
* @param dev Device instance.
* @param delay Minimum amount of time before triggering the action.
*
* @retval 0 If it succeeds. In case device runtime PM is not enabled or not
* available this function will be a no-op and will also return 0.
Expand All @@ -140,7 +141,7 @@ int pm_device_runtime_put(const struct device *dev);
*
* @see pm_device_runtime_put()
*/
int pm_device_runtime_put_async(const struct device *dev);
int pm_device_runtime_put_async(const struct device *dev, k_timeout_t delay);

/**
* @brief Check if device runtime is enabled for a given device.
Expand Down Expand Up @@ -188,9 +189,11 @@ static inline int pm_device_runtime_put(const struct device *dev)
return 0;
}

static inline int pm_device_runtime_put_async(const struct device *dev)
static inline int pm_device_runtime_put_async(const struct device *dev,
k_timeout_t delay)
{
ARG_UNUSED(dev);
ARG_UNUSED(delay);
return 0;
}

Expand Down
6 changes: 4 additions & 2 deletions include/zephyr/tracing/tracing.h
View file
Open in desktop
Expand Up @@ -1954,15 +1954,17 @@
/**
* @brief Trace putting a device (asynchronously) call entry.
* @param dev Device instance.
* @param delay Time to delay the operation
*/
#define sys_port_trace_pm_device_runtime_put_async_enter(dev)
#define sys_port_trace_pm_device_runtime_put_async_enter(dev, delay)

/**
* @brief Trace putting a device (asynchronously) call exit.
* @param dev Device instance.
* @param delay Time to delay the operation.
* @param ret Return value.
*/
#define sys_port_trace_pm_device_runtime_put_async_exit(dev, ret)
#define sys_port_trace_pm_device_runtime_put_async_exit(dev, delay, ret)

/**
* @brief Trace enabling device runtime PM call entry.
Expand Down
2 changes: 1 addition & 1 deletion soc/arm/st_stm32/stm32f4/Kconfig.defconfig.series
View file
Open in desktop
Expand Up @@ -19,6 +19,6 @@ config TASK_WDT_HW_FALLBACK_DELAY

config PM
select COUNTER
select COUNTER_RTC_STM32_SUBSECONDS
select COUNTER_RTC_STM32_SUBSECONDS if DT_HAS_ST_STM32_RTC_ENABLED
Copy link
Member

Choose a reason for hiding this comment

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

Can you explain when you faced this issue ?
In theory this shouldn't be required as rtc node should be enabled when CONFIG_PM=y.
It might not be the case when running generic PM tests for instance. But in that case we should ensure these tests are only ran on PM pre configured targets (with the required node enabled).

So I'd prefer you remove this, and we fix the board configuration issue.

Copy link
Member Author

Choose a reason for hiding this comment

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

this is broken in HEAD. Just try to enable PM for these targets e.g:

ceolin@vault:/mnt/nvme/zephyrproject/zephyr$ west build -p -b mikroe_mini_m4_for_stm32 samples/hello_world/ -- -DCONFIG_PM=y
-- west build: making build dir /mnt/nvme/zephyrproject/zephyr/build pristine
-- west build: generating a build system
Loading Zephyr default modules (Zephyr base).
-- Application: /mnt/nvme/zephyrproject/zephyr/samples/hello_world
-- CMake version: 3.25.1
-- Found Python3: /usr/bin/python3 (found suitable version "3.11.2", minimum required is "3.8") found components: Interpreter 
-- Cache files will be written to: /home/ceolin/.cache/zephyr
-- Zephyr version: 3.5.99 (/mnt/nvme/zephyrproject/zephyr)
-- Found west (found suitable version "1.1.0", minimum required is "0.14.0")
-- Board: mikroe_mini_m4_for_stm32
-- Found host-tools: zephyr 0.16.3 (/mnt/nvme/zephyrproject/zephyr-sdk-0.16.3)
-- Found toolchain: zephyr 0.16.3 (/mnt/nvme/zephyrproject/zephyr-sdk-0.16.3)
-- Found Dtc: /mnt/nvme/zephyrproject/zephyr-sdk-0.16.3/sysroots/x86_64-pokysdk-linux/usr/bin/dtc (found suitable version "1.6.0", minimum required is "1.4.6") 
-- Found BOARD.dts: /mnt/nvme/zephyrproject/zephyr/boards/arm/mikroe_mini_m4_for_stm32/mikroe_mini_m4_for_stm32.dts
-- Generated zephyr.dts: /mnt/nvme/zephyrproject/zephyr/build/zephyr/zephyr.dts
-- Generated devicetree_generated.h: /mnt/nvme/zephyrproject/zephyr/build/zephyr/include/generated/devicetree_generated.h
-- Including generated dts.cmake file: /mnt/nvme/zephyrproject/zephyr/build/zephyr/dts.cmake

warning: COUNTER_RTC_STM32_SUBSECONDS (defined at drivers/counter/Kconfig.stm32_rtc:47) has direct dependencies !SOC_SERIES_STM32F1X && COUNTER_RTC_STM32 && COUNTER with value n, but is currently being y-selected by the following symbols:
 - PM (defined at soc/arm/silabs_exx32/efr32bg22/Kconfig.defconfig.series:18, soc/arm/silabs_exx32/efr32bg27/Kconfig.defconfig.series:18, soc/arm/silabs_exx32/efr32mg24/Kconfig.defconfig.series:19, soc/arm/st_stm32/stm32f4/Kconfig.defconfig.series:20, subsys/pm/Kconfig:13), with value y, direct dependencies (SOC_SERIES_EFR32BG22 && SOC_FAMILY_EXX32) || (SOC_SERIES_EFR32BG27 && SOC_FAMILY_EXX32) || (SOC_SERIES_EFR32MG24 && SOC_FAMILY_EXX32) || SOC_SERIES_STM32F4X || (SYS_CLOCK_EXISTS && HAS_PM) (value: y), and select condition SOC_SERIES_STM32F4X (value: y)
Parsing /mnt/nvme/zephyrproject/zephyr/Kconfig
Loaded configuration '/mnt/nvme/zephyrproject/zephyr/boards/arm/mikroe_mini_m4_for_stm32/mikroe_mini_m4_for_stm32_defconfig'
Merged configuration '/mnt/nvme/zephyrproject/zephyr/samples/hello_world/prj.conf'
Merged configuration '/mnt/nvme/zephyrproject/zephyr/build/zephyr/misc/generated/extra_kconfig_options.conf'

error: Aborting due to Kconfig warnings

CMake Error at /mnt/nvme/zephyrproject/zephyr/cmake/modules/kconfig.cmake:355 (message):
  command failed with return code: 1
Call Stack (most recent call first):
  /mnt/nvme/zephyrproject/zephyr/cmake/modules/zephyr_default.cmake:129 (include)
  /mnt/nvme/zephyrproject/zephyr/share/zephyr-package/cmake/ZephyrConfig.cmake:66 (include)
  /mnt/nvme/zephyrproject/zephyr/share/zephyr-package/cmake/ZephyrConfig.cmake:92 (include_boilerplate)
  CMakeLists.txt:5 (find_package)

Copy link
Member

Choose a reason for hiding this comment

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

Being fixed by #66359


endif # SOC_SERIES_STM32F4X
32 changes: 24 additions & 8 deletions subsys/pm/device_runtime.c
View file
Open in desktop
Expand Up @@ -35,14 +35,16 @@ LOG_MODULE_DECLARE(pm_device, CONFIG_PM_DEVICE_LOG_LEVEL);
*
* @param dev Device instance.
* @param async Perform operation asynchronously.
* @param delay Period to delay the asynchronous operation.
*
* @retval 0 If device has been suspended or queued for suspend.
* @retval -EALREADY If device is already suspended (can only happen if get/put
* calls are unbalanced).
* @retval -EBUSY If the device is busy.
* @retval -errno Other negative errno, result of the action callback.
*/
static int runtime_suspend(const struct device *dev, bool async)
static int runtime_suspend(const struct device *dev, bool async,
k_timeout_t delay)
{
int ret = 0;
struct pm_device *pm = dev->pm;
Expand Down Expand Up @@ -77,7 +79,7 @@ static int runtime_suspend(const struct device *dev, bool async)
if (async && !k_is_pre_kernel()) {
/* queue suspend */
pm->state = PM_DEVICE_STATE_SUSPENDING;
(void)k_work_schedule(&pm->work, K_NO_WAIT);
(void)k_work_schedule(&pm->work, delay);
} else {
/* suspend now */
ret = pm->action_cb(pm->dev, PM_DEVICE_ACTION_SUSPEND);
Expand Down Expand Up @@ -177,8 +179,22 @@ int pm_device_runtime_get(const struct device *dev)

pm->usage++;

/*
* Check if the device has a pending suspend operation (not started
* yet) and cancel it. This way we avoid unnecessary operations because
* the device is actually active.
*/
if ((pm->state == PM_DEVICE_STATE_SUSPENDING) &&
((k_work_cancel_delayable(&pm->work) & K_WORK_RUNNING) == 0)) {
pm->state = PM_DEVICE_STATE_ACTIVE;
goto unlock;
}

if (!k_is_pre_kernel()) {
/* wait until possible async suspend is completed */
/*
* If the device is already suspending there is
* nothing else we can do but wait until it finishes.
*/
while (pm->state == PM_DEVICE_STATE_SUSPENDING) {
k_sem_give(&pm->lock);

Expand Down Expand Up @@ -221,7 +237,7 @@ int pm_device_runtime_put(const struct device *dev)
}

SYS_PORT_TRACING_FUNC_ENTER(pm, device_runtime_put, dev);
ret = runtime_suspend(dev, false);
ret = runtime_suspend(dev, false, K_NO_WAIT);

/*
* Now put the domain
Expand All @@ -235,17 +251,17 @@ int pm_device_runtime_put(const struct device *dev)
return ret;
}

int pm_device_runtime_put_async(const struct device *dev)
int pm_device_runtime_put_async(const struct device *dev, k_timeout_t delay)
{
int ret;

if (dev->pm == NULL) {
return 0;
}

SYS_PORT_TRACING_FUNC_ENTER(pm, device_runtime_put_async, dev);
ret = runtime_suspend(dev, true);
SYS_PORT_TRACING_FUNC_EXIT(pm, device_runtime_put_async, dev, ret);
SYS_PORT_TRACING_FUNC_ENTER(pm, device_runtime_put_async, dev, delay);
ret = runtime_suspend(dev, true, delay);
SYS_PORT_TRACING_FUNC_EXIT(pm, device_runtime_put_async, dev, delay, ret);

return ret;
}
Expand Down
4 changes: 2 additions & 2 deletions subsys/tracing/ctf/tracing_ctf.h
View file
Open in desktop
Expand Up @@ -338,8 +338,8 @@ extern "C" {
#define sys_port_trace_pm_device_runtime_get_exit(dev, ret)
#define sys_port_trace_pm_device_runtime_put_enter(dev)
#define sys_port_trace_pm_device_runtime_put_exit(dev, ret)
#define sys_port_trace_pm_device_runtime_put_async_enter(dev)
#define sys_port_trace_pm_device_runtime_put_async_exit(dev, ret)
#define sys_port_trace_pm_device_runtime_put_async_enter(dev, delay)
#define sys_port_trace_pm_device_runtime_put_async_exit(dev, delay, ret)
#define sys_port_trace_pm_device_runtime_enable_enter(dev)
#define sys_port_trace_pm_device_runtime_enable_exit(dev, ret)
#define sys_port_trace_pm_device_runtime_disable_enter(dev)
Expand Down
2 changes: 1 addition & 1 deletion subsys/tracing/sysview/SYSVIEW_Zephyr.txt
View file
Open in desktop
Expand Up @@ -164,7 +164,7 @@ TaskState 0xBF 1=dummy, 2=Waiting, 4=New, 8=Terminated, 16=Suspended, 32=Termina
156 pm_system_suspend ticks=%u | Returns %Bool
157 pm_device_runtime_get dev=%I | Returns %u
158 pm_device_runtime_put dev=%I | Returns %u
159 pm_device_runtime_put_async dev=%I | Returns %u
159 pm_device_runtime_put_async dev=%I, Delay=%TimeOut | Returns %u
160 pm_device_runtime_enable dev=%I | Returns %u
161 pm_device_runtime_disable dev=%I | Returns %u

Expand Down
6 changes: 3 additions & 3 deletions subsys/tracing/sysview/tracing_sysview.h
View file
Open in desktop
Expand Up @@ -640,10 +640,10 @@ void sys_trace_k_thread_info(struct k_thread *thread);
#define sys_port_trace_pm_device_runtime_put_exit(dev, ret) \
SEGGER_SYSVIEW_RecordEndCallU32(TID_PM_DEVICE_RUNTIME_PUT, \
(uint32_t)ret)
#define sys_port_trace_pm_device_runtime_put_async_enter(dev) \
#define sys_port_trace_pm_device_runtime_put_async_enter(dev, delay) \
SEGGER_SYSVIEW_RecordU32(TID_PM_DEVICE_RUNTIME_PUT_ASYNC, \
(uint32_t)(uintptr_t)dev)
#define sys_port_trace_pm_device_runtime_put_async_exit(dev, ret) \
(uint32_t)(uintptr_t)dev, (uint32_t)delay.ticks)
#define sys_port_trace_pm_device_runtime_put_async_exit(dev, delay, ret) \
SEGGER_SYSVIEW_RecordEndCallU32(TID_PM_DEVICE_RUNTIME_PUT_ASYNC, \
(uint32_t)ret)
#define sys_port_trace_pm_device_runtime_enable_enter(dev) \
Expand Down
4 changes: 2 additions & 2 deletions subsys/tracing/test/tracing_test.h
View file
Open in desktop
Expand Up @@ -442,8 +442,8 @@
#define sys_port_trace_pm_device_runtime_get_exit(dev, ret)
#define sys_port_trace_pm_device_runtime_put_enter(dev)
#define sys_port_trace_pm_device_runtime_put_exit(dev, ret)
#define sys_port_trace_pm_device_runtime_put_async_enter(dev)
#define sys_port_trace_pm_device_runtime_put_async_exit(dev, ret)
#define sys_port_trace_pm_device_runtime_put_async_enter(dev, delay)
#define sys_port_trace_pm_device_runtime_put_async_exit(dev, delay, ret)
#define sys_port_trace_pm_device_runtime_enable_enter(dev)
#define sys_port_trace_pm_device_runtime_enable_exit(dev, ret)
#define sys_port_trace_pm_device_runtime_disable_enter(dev)
Expand Down
4 changes: 2 additions & 2 deletions subsys/tracing/user/tracing_user.h
View file
Open in desktop
Expand Up @@ -329,8 +329,8 @@ void sys_trace_idle(void);
#define sys_port_trace_pm_device_runtime_get_exit(dev, ret)
#define sys_port_trace_pm_device_runtime_put_enter(dev)
#define sys_port_trace_pm_device_runtime_put_exit(dev, ret)
#define sys_port_trace_pm_device_runtime_put_async_enter(dev)
#define sys_port_trace_pm_device_runtime_put_async_exit(dev, ret)
#define sys_port_trace_pm_device_runtime_put_async_enter(dev, delay)
#define sys_port_trace_pm_device_runtime_put_async_exit(dev, delay, ret)
#define sys_port_trace_pm_device_runtime_enable_enter(dev)
#define sys_port_trace_pm_device_runtime_enable_exit(dev, ret)
#define sys_port_trace_pm_device_runtime_disable_enter(dev)
Expand Down
49 changes: 45 additions & 4 deletions tests/subsys/pm/device_runtime_api/src/main.c
View file
Open in desktop
Expand Up @@ -42,7 +42,7 @@ void test_api_setup(void *data)
zassert_equal(ret, 0);
ret = pm_device_runtime_put(test_dev);
zassert_equal(ret, 0);
ret = pm_device_runtime_put_async(test_dev);
ret = pm_device_runtime_put_async(test_dev, K_NO_WAIT);
zassert_equal(ret, 0);

/* enable runtime PM */
Expand Down Expand Up @@ -138,7 +138,7 @@ ZTEST(device_runtime_api, test_api)
test_driver_pm_async(test_dev);

/* usage: 1, -1, suspend: yes (queued) */
ret = pm_device_runtime_put_async(test_dev);
ret = pm_device_runtime_put_async(test_dev, K_NO_WAIT);
zassert_equal(ret, 0);

(void)pm_device_state_get(test_dev, &state);
Expand All @@ -149,7 +149,7 @@ ZTEST(device_runtime_api, test_api)
zassert_equal(ret, -EALREADY);

/* usage: 0, -1, suspend: no (unbalanced call) */
ret = pm_device_runtime_put_async(test_dev);
ret = pm_device_runtime_put_async(test_dev, K_NO_WAIT);
zassert_equal(ret, -EALREADY);

/* unblock test driver and let it finish */
Expand All @@ -171,7 +171,7 @@ ZTEST(device_runtime_api, test_api)
test_driver_pm_async(test_dev);

/* usage: 1, -1, suspend: yes (queued) */
ret = pm_device_runtime_put_async(test_dev);
ret = pm_device_runtime_put_async(test_dev, K_NO_WAIT);
zassert_equal(ret, 0);

(void)pm_device_state_get(test_dev, &state);
Expand Down Expand Up @@ -200,6 +200,47 @@ ZTEST(device_runtime_api, test_api)
(void)pm_device_state_get(test_dev, &state);
zassert_equal(state, PM_DEVICE_STATE_ACTIVE);

/* Test if getting a device before an async operation starts does
* not trigger any device pm action.
*/
size_t count = test_driver_pm_count(test_dev);

ret = pm_device_runtime_put_async(test_dev, K_MSEC(10));
zassert_equal(ret, 0);

(void)pm_device_state_get(test_dev, &state);
zassert_equal(state, PM_DEVICE_STATE_SUSPENDING);

ret = pm_device_runtime_get(test_dev);
zassert_equal(ret, 0);

/* Now lets check if the calls above have triggered a device
* pm action
*/
zassert_equal(count, test_driver_pm_count(test_dev));

/*
* test if async put with a delay respects the given time.
*/
ret = pm_device_runtime_put_async(test_dev, K_MSEC(100));

(void)pm_device_state_get(test_dev, &state);
zassert_equal(state, PM_DEVICE_STATE_SUSPENDING);

k_sleep(K_MSEC(80));

/* It should still be suspending since we have waited less than
* the delay we've set.
*/
(void)pm_device_state_get(test_dev, &state);
zassert_equal(state, PM_DEVICE_STATE_SUSPENDING);

k_sleep(K_MSEC(30));

/* Now it should be already suspended */
(void)pm_device_state_get(test_dev, &state);
zassert_equal(state, PM_DEVICE_STATE_SUSPENDED);

/* Put operation should fail due the state be locked. */
ret = pm_device_runtime_disable(test_dev);
zassert_equal(ret, 0);
Expand Down
10 changes: 10 additions & 0 deletions tests/subsys/pm/device_runtime_api/src/test_driver.c
View file
Open in desktop
Expand Up @@ -10,6 +10,7 @@
#include <zephyr/pm/device.h>

struct test_driver_data {
size_t count;
bool ongoing;
bool async;
struct k_sem sync;
Expand All @@ -29,6 +30,8 @@ static int test_driver_action(const struct device *dev,

data->ongoing = false;

data->count++;

return 0;
}

Expand All @@ -53,6 +56,13 @@ bool test_driver_pm_ongoing(const struct device *dev)
return data->ongoing;
}

size_t test_driver_pm_count(const struct device *dev)
{
struct test_driver_data *data = dev->data;

return data->count;
}

int test_driver_init(const struct device *dev)
{
struct test_driver_data *data = dev->data;
Expand Down
9 changes: 9 additions & 0 deletions tests/subsys/pm/device_runtime_api/src/test_driver.h
View file
Open in desktop
Expand Up @@ -36,4 +36,13 @@ void test_driver_pm_done(const struct device *dev);
*/
bool test_driver_pm_ongoing(const struct device *dev);

/**
* @brief Gets the number of times the device changed state.
*
* @param dev Device instance.
*
* @return The number of state changes the device made.
*/
size_t test_driver_pm_count(const struct device *dev);

#endif /* TESTS_SUBSYS_PM_DEVICE_RUNTIME_TEST_DRIVER_H_ */