Skip to content

[BUG] IPC timeout check mismatch between kernel and fw #8115

Closed

Description

@RanderWang

Linux driver set ipc timeout duration to 500ms, but FW wait 5000ms in cpu_disable_core for SET_DX ipc message. This makes driver stop but fw is still waiting. And why we need 5000ms to disable core 1 ~ n ? Can we adopt 400 or 300 ms ?

void cpu_disable_core(int id) {
       .....
	uint64_t timeout = k_cycle_get_64() +
		k_ms_to_cyc_ceil64(CONFIG_SECONDARY_CORE_DISABLING_TIMEOUT); // 5000ms

	/* Waiting for secondary core to enter idle state */
	while (arch_cpu_active(id) && (k_cycle_get_64() < timeout))
		idelay(PLATFORM_DEFAULT_DELAY);

	if (arch_cpu_active(id)) {
		tr_err(&zephyr_tr, "core %d did not enter idle state", id);
		return;
	}
     .....
}

Activity

  1. added
    bugSomething isn't working as expected
    on Aug 29, 2023
  2. added
    P2Critical bugs or normal features
    on Aug 29, 2023
  3. kv2019i commented on Aug 29, 2023

    @kv2019i
    Collaborator

    Labeling P2 for investigation, need to check with git blame on what is history of this long timeout.

  4. RanderWang commented on Aug 29, 2023

    @RanderWang
    CollaboratorAuthor

    normally only needs less than 1ms

    [    9.832556] <err> zephyr: cpu_disable_core: ~~~~~~~~ core[2] entry
    [    9.832568] <inf> os: pm_state_set: --------------- power off core[2] 
    [    9.832585] <err> zephyr: cpu_disable_core: ^^^^^^^^^^ core[2] exit
    
    [    9.835976] <err> zephyr: cpu_disable_core: ~~~~~~~~ core[1] entry
    [    9.835986] <inf> os: pm_state_set: --------------- power off core[1] 
    [    9.836003] <err> zephyr: cpu_disable_core: ^^^^^^^^^^ core[1] exit
    
  5. kv2019i commented on Aug 29, 2023

    @kv2019i
    Collaborator

    @RanderWang git blame is your friend.. looking at history, the 5000 comes from this commit from @tmleman

    commit b6a4bb682cade0f4eb7a9ffe872f6fe46e099a2e
    Author: Tomasz Leman <tomasz.m.leman@intel.com>
    Date:   Fri Apr 29 16:37:07 2022 +0200
    
        platform: Kconfig: add timeout for disabling secondary core
        
        Count of loops used when waiting for secondary core to enter OFF state
        in transition to D3 state.
    

    This is a generic setting applying to all platforms, so it's probably conservative because of that. Do you @tmleman any specific reason to have such a long time.

    We should reduce this to under 500ms as this will now complicated debug. If the secondary core power-down fails, the kernel will hit IPC timeout sooner (and bring down the DSP as error recovery). It'd be better to error out on the FW side and return a proper error that can be displayed in kernel log.

  6. tmleman commented on Aug 29, 2023

    @tmleman
    Contributor

    Do you @tmleman any specific reason to have such a long time.

    @kv2019i timeout is set so high to avoid a situation where it is constantly being increased. It should never get to a timeout, so such a high value seemed appropriate. If we want to reduce this value, we should bear in mind that in the development environment (FPGA) this value will probably be higher than on silicon.

  7. lgirdwood commented on Aug 30, 2023

    @lgirdwood
    Member

    It looks like we need a Kconfig debug option were we multiple all delays by 10x (or whatever) to cater for all different development environments.
    5 seconds is too long today as this could block userspace (frozen app on audio driver state transition). Can we lower to the real limit.

  8. RanderWang commented on Aug 31, 2023

    @RanderWang
    CollaboratorAuthor

    It looks like we need a Kconfig debug option were we multiple all delays by 10x (or whatever) to cater for all different development environments. 5 seconds is too long today as this could block userspace (frozen app on audio driver state transition). Can we lower to the real limit.

    It is configurable in Kconfig. The default value is 5000, so we can change it when building fw

  9. kv2019i commented on Aug 31, 2023

    @kv2019i
    Collaborator

    I think we should change the value for at least Intel platforms to be smaller than the IPC timeout in upstream Linux driver. On special systems, both host and FW timeouts need/can be increased but defaults should be aligned so that FW will return error before kernel times out on the IPC completely.

  10. plbossart commented on Aug 31, 2023

    @plbossart
    Member

    All the timeouts we use in the kernel are defined based on the recommendation of hardware folks. Specifically for SPA/CPA power-up, we have a recommendation to loop 100 times and wait for 10usec each time.

    Clearly 5s timeout for core power management was not a hardware recommendation....

  11. lgirdwood commented on Sep 11, 2023

    @lgirdwood
    Member

    @mwasko fyi needed for mtl06

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    IPC timeoutIPC timeout observedMTLApplies to Meteor Lake platformP2Critical bugs or normal featuresbugSomething isn't working as expected

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions