> 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?
> 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?
> + */
> + 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.
> + msg.ext.set_d0ix.wake = enable_pg;
simplify the argument? Not sure anyone could understand what wake and
enable_pg mean.
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.
> + } 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?
> +} __packed;
> +
> +int avs_ipc_set_dx(struct avs_dev *adev, u32 core_mask, bool powerup);
> +int avs_ipc_set_d0ix(struct avs_dev *adev, bool enable_pg, bool streaming);
> +
> #endif /* __SOUND_SOC_INTEL_AVS_MSGS_H */
