On 2022-04-26 11:58 PM, Pierre-Louis Bossart wrote:
On 4/26/22 12:23, Cezary Rojewski wrote:
Audio DSP device supports D0 substates in form of D0ix, allowing for
preserving more power even when device is still considered active (D0).
When entered, certain domains which are not being currently used become
power gated. Entering and leaving D0ix is a complex process and differs
between firmware generations.
Conditions that disallow D0i3 and require immediate D0i0 transition
include but may not be limited to: IPC traffic, firmware tracing and
SRAM I/O. To make D0ix toggling sane, delay D0i3 transition and refresh
the timer each time an IPC is requrested.
typo: requested.
Ack.
I find it odd to list all kinds of criteria but only handle one in the end. Do the other matter, is this a TODO, unclear what you are trying to say.
Good question. Firmware tracing code is part of debugfs.c file which has
not yet been shared. But all other usages, not listed here, come down to
invoking enable_d0ix() or disable_d0ix() whenever given operation blocks
DSP from transitioning to D0iX.
Other usages such as directly accessing SRAM (outside of IPC handling)
is non-existant in the avs-driver. When IPCs, most firmware generations
take care of toggling d0ix for you.
int avs_get_module_entry(struct avs_dev *adev, const guid_t *uuid, struct avs_module_entry *entry);
diff --git a/sound/soc/intel/avs/dsp.c b/sound/soc/intel/avs/dsp.c
index 3ff17bd22a5a..2f18b137ff42 100644
--- a/sound/soc/intel/avs/dsp.c
+++ b/sound/soc/intel/avs/dsp.c
@@ -152,6 +152,15 @@ static int avs_dsp_get_core(struct avs_dev *adev, u32 core_id)
adev->core_refs[core_id]++;
if (adev->core_refs[core_id] == 1) {
+ /*
+ * No cores other than main-core can be running for DSP
+ * to achieve d0ix. Conscious SET_D0IX IPC failure is permitted,
conscious failure? what's that?
Any IPC failure which does not end in firmware throwing an exception or
failing to deliver the response (IPC timeout). Sending response with
status=<some error> is still a valid response.
+ * simply d0ix power state will no longer be attempted.
+ */
+ ret = avs_dsp_disable_d0ix(adev);
+ if (ret && ret != -AVS_EIPC)
+ goto err_disable_d0ix;
+
ret = avs_dsp_enable(adev, mask);
if (ret)
goto err_enable_dsp;
tatic int
+avs_dsp_set_d0ix(struct avs_dev *adev, bool enable)
+{
+ struct avs_ipc *ipc = adev->ipc;
+ int ret;
+
+ /* Is transition required? */
+ if (ipc->in_d0ix == enable)
+ return 0;
+
+ ret = avs_dsp_op(adev, set_d0ix, enable);
+ if (ret) {
+ /* Prevent further d0ix attempts on conscious IPC failure. */
??
Same as above but as I'm not sure whether '??' relates to comment above
or the usage of 'conscious' word, I'll add to that:
To improve user-experience, we block any d0ix further d0ix attempts if
even one SET_D0IX IPC fails. Audio can be streamed just fine without
d0ix substate albeit it might not be as power efficient as with
transition enabled.
+ if (ret == -AVS_EIPC)
+ atomic_inc(&ipc->d0ix_disable_depth);
+
+ ipc->in_d0ix = false;
+ return ret;
+ }
+
+ ipc->in_d0ix = enable;
+ return 0;
+}
