>
>>> + 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.
again please do not assume that anyone reviewing this code has access to
the firmware specification.
>
>> 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.
it's not about judging, just explaining what is expected on the firmware
side and what the driver needs to do.
>
> '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.
