Enable ipc4 d0i3 setting #3928

RanderWang · 2022-10-14T09:19:45Z

Add support of d0i3 setting for ipc4

Tested on ADL and MTL RVP, IPC msg was successful and no error was found. To fully validate d0i3 effect we need to prepare a mature environment to check power state

sound/soc/sof/intel/hda-dsp.c

sound/soc/sof/ipc3.c

sound/soc/sof/ipc4.c

sound/soc/sof/intel/hda-dsp.c

sound/soc/sof/intel/cnl.c

sound/soc/sof/ipc4.c

sound/soc/sof/intel/hda-dsp.c

RanderWang · 2022-10-19T08:28:22Z

SOFCI TEST

RanderWang · 2022-10-19T13:19:48Z

@plbossart @ujfalusi @ranj063 could you help to review this PR ? Thanks!

sound/soc/sof/intel/hda-dsp.c

sound/soc/sof/ipc3.c

sound/soc/sof/intel/hda-dsp.c

sound/soc/sof/intel/cnl.c

RanderWang · 2022-10-24T08:58:44Z

@ujfalusi @bardliao updated, thanks for your comments!

sound/soc/sof/sof-priv.h

sound/soc/sof/intel/hda-dsp.c

sound/soc/sof/ipc4.c

sound/soc/sof/intel/hda-dsp.c

plbossart

The order of patches doesn't seem quite right, the logic of the D0i3 streaming is still odd to me, and last I don't get what the D0I3C/IPC order should be.

sound/soc/sof/intel/hda-dsp.c

sound/soc/sof/sof-priv.h

sound/soc/sof/intel/hda-dsp.c

sound/soc/sof/ipc4.c

sound/soc/sof/intel/hda-ipc.c

sound/soc/sof/intel/hda-dsp.c

sound/soc/sof/intel/hda-ipc.c

sound/soc/sof/intel/hda-dsp.c

plbossart · 2022-11-03T14:20:39Z

In my test the different order get the same result on MTL. Since pmc driver on mtl is not ready so we can't make any conclusion. Two month ago, I discussed with keyon about d0i3 register setting in host and have a same idea that: d0i3 register is only a notification for pmc, not a switch to enable or disable. According to spec, the power saving is mainly done in audio hw component, not by pmc. It is possible that the order doesn't matter even pmc driver is ready. But the order in this PR is comfortable to me.

From what I remember of my younger days, the drivers can signal to PMC they are 'D0i3' ready. The PMC may or may not make the transition, depending on other factors. If we don't have any evidence that the order matters, then we should keep the existing code as is. "If it ain't broke don't fix it".

ranj063

Thanks @RanderWang . Just a word of caution that if #3986 gets merged first which it looks it will, you will have to use the new ops lookup function that @ujfalusi has introduced in this PR after the rebase.

RanderWang · 2022-11-04T01:17:28Z

Thanks @RanderWang . Just a word of caution that if #3986 gets merged first which it looks it will, you will have to use the new ops lookup function that @ujfalusi has introduced in this PR after the rebase.

sure, thanks!

ujfalusi · 2022-11-03T13:08:04Z

sound/soc/sof/sof-priv.h

what is "pm gate"?

I don't know the exactly meaning. It is used for a long time

static int hda_dsp_send_pm_gate_ipc(struct snd_sof_dev *sdev, u32 flags) { struct sof_ipc_pm_gate pm_gate; struct sof_ipc_reply reply; memset(&pm_gate, 0, sizeof(pm_gate)); /* configure pm_gate ipc message */ pm_gate.hdr.size = sizeof(pm_gate); pm_gate.hdr.cmd = SOF_IPC_GLB_PM_MSG | SOF_IPC_PM_GATE; pm_gate.flags = flags; /* send pm_gate ipc to dsp */ return sof_ipc_tx_message_no_pm(sdev->ipc, &pm_gate, sizeof(pm_gate), &reply, sizeof(reply)); }

The terminology comes form the firmware which uses the flags to determine which SRAMS to pwoer gate or not depending on whats active

ujfalusi

@RanderWang, tentatively approved.
Can you answer the hda_dsp_d0i3_streaming_applicable() vs snd_sof_dsp_only_d0i3_compatible_stream_active()

What is the expectation if we have a single d0i3 compatible capture running without any playback? Does that should be allowed?

sound/soc/sof/intel/hda-dsp.c

RanderWang · 2022-11-07T05:28:37Z

@RanderWang, tentatively approved. Can you answer the hda_dsp_d0i3_streaming_applicable() vs snd_sof_dsp_only_d0i3_compatible_stream_active()

What is the expectation if we have a single d0i3 compatible capture running without any playback? Does that should be allowed?

we have D0i3-hotwording (capture), which works WITHOUT any host DMA transfers while in D0i3, and D0i3-streaming (playback with deep buffer) which requires host DMA transfers to work in D0i3 - albeit in bursts. They share the pm setting except host dma . snd_sof_dsp_only_d0i3_compatible_stream_active is to check whether audio hardware can enter d0i3 state, hda_dsp_d0i3_streaming_applicable is checking whether audio hardware can enter d0i3 state with host dma enabled.

we can have a single d0i3 compatible capture without playback, keyword detection. This is a feature enabled on TGL.

Set_pm_gate depends on ipc version. This patch defines the ops for both IPC3 and IPC4. Signed-off-by: Rander Wang <rander.wang@intel.com>

Use set_pm_gate to unify pm gate setting for different ipc version. Signed-off-by: Rander Wang <rander.wang@intel.com>

The driver shall update the power state to D0i0 before sending a generic IPC. Power-related IPCs are the exception to the rule, they may be sent even when the power-state is D0i3 Signed-off-by: Rander Wang <rander.wang@intel.com>

Schedule a delayed work for d0i3 entry after every non-pm ipc msg. Signed-off-by: Rander Wang <rander.wang@intel.com>

Enable d0i3 streaming if all the active streams can work in d0i3 state and playback is enabled. Signed-off-by: Rander Wang <rander.wang@intel.com>

RanderWang · 2022-11-07T05:50:53Z

updated to use new sof_ipc_get_ops() to get pm ops

plbossart

only 2 comments, which can both be addressed with a fixup! PR. I think it's good enough for now.

plbossart · 2022-11-09T01:15:56Z

sound/soc/sof/ipc3.c

 	return sof_ipc3_ctx_ipc(sdev, SOF_IPC_PM_CTX_RESTORE);
 }

+static int sof_ipc3_set_pm_gate(struct snd_sof_dev *sdev, u32 flags)


maybe rename to 'set_pm_gate_flags' - which seems to be a better definition of what the functions actually do.

It does a bit more than setting flags, it sends the message to firmware to set the 'pm_gate' and the state is described by the 'flags' parameter.
It is more like 'pm_gate_params' if we want to be so generic.

sound/soc/sof/intel/hda-ipc.c

sound/soc/sof/intel/hda-dsp.c

plbossart · 2022-11-09T15:08:17Z

@ujfalusi any comments?

ujfalusi · 2022-11-10T09:38:22Z

sound/soc/sof/ipc3.c

 	return sof_ipc3_ctx_ipc(sdev, SOF_IPC_PM_CTX_RESTORE);
 }

+static int sof_ipc3_set_pm_gate(struct snd_sof_dev *sdev, u32 flags)


It does a bit more than setting flags, it sends the message to firmware to set the 'pm_gate' and the state is described by the 'flags' parameter.
It is more like 'pm_gate_params' if we want to be so generic.

RanderWang requested review from ajitkupandey, bardliao, dbaluta, juimonen, kuanhsuncheng, kv2019i, lgirdwood, libinyang, lyakh, plbossart, ranj063, ujfalusi and yaochunhung as code owners October 14, 2022 09:19

ujfalusi requested changes Oct 14, 2022

View reviewed changes

sound/soc/sof/intel/hda-dsp.c Outdated Show resolved Hide resolved

sound/soc/sof/intel/hda-dsp.c Outdated Show resolved Hide resolved

sound/soc/sof/intel/hda-dsp.c Outdated Show resolved Hide resolved

RanderWang force-pushed the ipc4_d0i3_setting branch 2 times, most recently from 6169058 to 294ce04 Compare October 14, 2022 12:15

ranj063 reviewed Oct 14, 2022

View reviewed changes

RanderWang force-pushed the ipc4_d0i3_setting branch 2 times, most recently from 3c657d9 to 5de3e7a Compare October 17, 2022 07:12

RanderWang requested a review from ujfalusi October 19, 2022 08:31

bardliao reviewed Oct 20, 2022

View reviewed changes

sound/soc/sof/intel/hda-dsp.c Outdated Show resolved Hide resolved

bardliao reviewed Oct 20, 2022

View reviewed changes

sound/soc/sof/intel/hda-dsp.c Outdated Show resolved Hide resolved

ujfalusi reviewed Oct 20, 2022

View reviewed changes

RanderWang force-pushed the ipc4_d0i3_setting branch 2 times, most recently from f9595ee to c714320 Compare October 24, 2022 08:06

ujfalusi requested changes Oct 24, 2022

View reviewed changes

sound/soc/sof/sof-priv.h Outdated Show resolved Hide resolved

sound/soc/sof/intel/hda-dsp.c Outdated Show resolved Hide resolved

sound/soc/sof/ipc4.c Outdated Show resolved Hide resolved

bardliao reviewed Oct 24, 2022

View reviewed changes

sound/soc/sof/intel/hda-dsp.c Outdated Show resolved Hide resolved

plbossart requested changes Nov 2, 2022

View reviewed changes

ranj063 reviewed Nov 2, 2022

View reviewed changes

sound/soc/sof/intel/hda-dsp.c Outdated Show resolved Hide resolved

sound/soc/sof/intel/hda-ipc.c Outdated Show resolved Hide resolved

sound/soc/sof/intel/hda-dsp.c Outdated Show resolved Hide resolved

RanderWang force-pushed the ipc4_d0i3_setting branch from 427bc51 to f16a81c Compare November 3, 2022 08:34

RanderWang force-pushed the ipc4_d0i3_setting branch from f16a81c to a93ee35 Compare November 4, 2022 00:50

ranj063 previously approved these changes Nov 4, 2022

View reviewed changes

bardliao previously approved these changes Nov 4, 2022

View reviewed changes

ujfalusi reviewed Nov 4, 2022

View reviewed changes

ujfalusi previously approved these changes Nov 4, 2022

View reviewed changes

sound/soc/sof/intel/hda-dsp.c Outdated Show resolved Hide resolved

RanderWang added 5 commits November 7, 2022 13:40

ASoC: SOF: Introduce a new set_pm_gate() IPC PM op

ed62ae4

Set_pm_gate depends on ipc version. This patch defines the ops for both IPC3 and IPC4. Signed-off-by: Rander Wang <rander.wang@intel.com>

ASoC: SOF: use set_pm_gate according to ipc version

2f3d78e

Use set_pm_gate to unify pm gate setting for different ipc version. Signed-off-by: Rander Wang <rander.wang@intel.com>

ASoC: SOF: wake up dsp core before sending ipc msg

cf0258d

The driver shall update the power state to D0i0 before sending a generic IPC. Power-related IPCs are the exception to the rule, they may be sent even when the power-state is D0i3 Signed-off-by: Rander Wang <rander.wang@intel.com>

ASoC: SOF: enable d0i3 work for ipc4

073343a

Schedule a delayed work for d0i3 entry after every non-pm ipc msg. Signed-off-by: Rander Wang <rander.wang@intel.com>

ASoC: SOF: set streaming flag for d0i3

30189ab

Enable d0i3 streaming if all the active streams can work in d0i3 state and playback is enabled. Signed-off-by: Rander Wang <rander.wang@intel.com>

RanderWang dismissed stale reviews from ujfalusi, bardliao, and ranj063 via 30189ab November 7, 2022 05:50

RanderWang force-pushed the ipc4_d0i3_setting branch from a93ee35 to 30189ab Compare November 7, 2022 05:50

plbossart approved these changes Nov 9, 2022

View reviewed changes

plbossart requested review from bardliao, ranj063 and ujfalusi November 9, 2022 01:21

ranj063 approved these changes Nov 9, 2022

View reviewed changes

ujfalusi approved these changes Nov 10, 2022

View reviewed changes

plbossart merged commit 7e9c225 into thesofproject:topic/sof-dev Nov 10, 2022

yongzhi1 mentioned this pull request Nov 15, 2022

[BUG] no sound coming from speakers when playing 16-bit clips on DeepBuffer PCM dev thesofproject/sof#6595

Closed

Enable ipc4 d0i3 setting #3928

Enable ipc4 d0i3 setting #3928

Uh oh!

Conversation

RanderWang commented Oct 14, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

RanderWang commented Oct 19, 2022

Uh oh!

RanderWang commented Oct 19, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

RanderWang commented Oct 24, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

plbossart left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

plbossart commented Nov 3, 2022 via email

Uh oh!

ranj063 left a comment

Choose a reason for hiding this comment

Uh oh!

RanderWang commented Nov 4, 2022

Uh oh!

ujfalusi Nov 3, 2022

Choose a reason for hiding this comment

Uh oh!

RanderWang Nov 4, 2022

Choose a reason for hiding this comment

Uh oh!

ranj063 Nov 4, 2022

Choose a reason for hiding this comment

Uh oh!

ujfalusi left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

RanderWang commented Nov 7, 2022

Uh oh!

RanderWang commented Nov 7, 2022

Uh oh!

plbossart left a comment

Choose a reason for hiding this comment

Uh oh!

plbossart Nov 9, 2022

Choose a reason for hiding this comment

Uh oh!