On 2022-02-25 2:37 AM, Pierre-Louis Bossart wrote:
Audio DSP supports low power states i.e.: transitions between D0 and D3
and D0-substates in form of D0i3. That process is a combination of core
D0i0 and D0i3?
Ack.
and IPC operations. Here, Dx and D0ix IPC handlers are added.
Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@xxxxxxxxxxxxxxx>
Signed-off-by: Cezary Rojewski <cezary.rojewski@xxxxxxxxx>
---
sound/soc/intel/avs/messages.c | 43 ++++++++++++++++++++++++++++++++++
sound/soc/intel/avs/messages.h | 16 +++++++++++++
2 files changed, 59 insertions(+)
diff --git a/sound/soc/intel/avs/messages.c b/sound/soc/intel/avs/messages.c
index e870d5792a77..1b589689410f 100644
--- a/sound/soc/intel/avs/messages.c
+++ b/sound/soc/intel/avs/messages.c
@@ -347,3 +347,46 @@ int avs_ipc_get_large_config(struct avs_dev *adev, u16 module_id, u8 instance_id
return 0;
}
+
+int avs_ipc_set_dx(struct avs_dev *adev, u32 core_mask, bool powerup)
+{
+ union avs_module_msg msg = AVS_MODULE_REQUEST(SET_DX);
+ struct avs_ipc_msg request;
+ struct avs_dxstate_info dx;
+ int ret;
+
+ dx.core_mask = core_mask;
+ dx.dx_mask = powerup ? core_mask : 0;
+ request.header = msg.val;
+ request.data = &dx;
+ request.size = sizeof(dx);
+
+ /*
+ * SET_D0 is sent for non-main cores only while SET_D3 is used to
+ * suspend for all of them. Both cases prevent any D0I3 transitions.
The asymmetry in the comment isn't clear. Did you mean that the main
code is in D0 when powered?
Yes. There is no putting MAIN_CORE to D0 as we must be in D0 to begin
with, if we're thinking about sending an IPC to the base firmware.
+ */
+ ret = avs_dsp_send_pm_msg(adev, &request, NULL, true);
+ if (ret)
+ avs_ipc_err(adev, &request, "set dx", ret);
+
+ return ret;
+}
+
+int avs_ipc_set_d0ix(struct avs_dev *adev, bool enable_pg, bool streaming)
+{
+ union avs_module_msg msg = AVS_MODULE_REQUEST(SET_D0IX);
+ struct avs_ipc_msg request = {0};
+ int ret;
+
+ /* Wake & streaming for < cAVS 2.0 */
I don't how anyone outside of Intel could understand this comment.
Consider explaining what the two terms refer to.
Sure, will improve the wording.
+ msg.ext.set_d0ix.wake = enable_pg;
simplify the argument? Not sure anyone could understand what wake and
enable_pg mean.
Well, CG and PG are popular shortcuts among Intel audio team and stand
for clock gating and power gating respectively. 'wake' is firmware
specific. I can provide a comment, but not all question are going to be
answered by it. Firmware specification is the place to find the answer
for most of these.
int avs_ipc_set_d0ix(struct avs_dev *adev, bool wake, bool streaming)
+ msg.ext.set_d0ix.streaming = streaming;
+
+ request.header = msg.val;
+
+ ret = avs_dsp_send_pm_msg(adev, &request, NULL, false);
+ if (ret)
+ avs_ipc_err(adev, &request, "set d0ix", ret);
+
+ return ret;
+}
diff --git a/sound/soc/intel/avs/messages.h b/sound/soc/intel/avs/messages.h
index 1dabd1005327..bbdba4631b1f 100644
--- a/sound/soc/intel/avs/messages.h
+++ b/sound/soc/intel/avs/messages.h
@@ -101,6 +101,8 @@ enum avs_module_msg_type {
AVS_MOD_LARGE_CONFIG_SET = 4,
AVS_MOD_BIND = 5,
AVS_MOD_UNBIND = 6,
+ AVS_MOD_SET_DX = 7,
+ AVS_MOD_SET_D0IX = 8,
AVS_MOD_DELETE_INSTANCE = 11,
};
@@ -137,6 +139,11 @@ union avs_module_msg {
u32 dst_queue:3;
u32 src_queue:3;
} bind_unbind;
+ struct {
+ /* cAVS < 2.0 */
+ u32 wake:1;
+ u32 streaming:1;
you probably want to explain how a 'streaming' flag is set at the module
level? One would think all modules connected in a pipeline would need to
use the same flag value.
Some of the fields are confusing and I agree, but driver has to adhere
to FW expectations if it wants to be a working one. I would like to
avoid judging the firmware architecture here, regardless of how
confusing we think it is.
'wake' and 'streaming' fields are part of SET_D0ix message is which part
of MODULE-type message interface. Base firmware is, from architecture
point of view, a module of type=0 (module_id) and instance id=0
(instance_id).
+ } set_d0ix;
} ext;
};
} __packed;
@@ -298,4 +305,13 @@ int avs_ipc_get_large_config(struct avs_dev *adev, u16 module_id, u8 instance_id
u8 param_id, u8 *request_data, size_t request_size,
u8 **reply_data, size_t *reply_size);
+/* DSP cores and domains power management messages */
+struct avs_dxstate_info {
+ u32 core_mask;
+ u32 dx_mask;
what is the convention for D0 and D3 in the mask ? which one is 0 or 1?
Is this also handled in a hierarchical way where only the bits set in
core_mask matter?
Can provide a short kernel-doc for these two, sure.
