Re: [PATCH v3 08/17] ASoC: Intel: avs: Add power management requests

[Cezary Rojewski <cezary.rojewski@intel.com>]

On 2022-03-04 5:24 PM, Ranjani Sridharan wrote:
> On Fri, 2022-03-04 at 15:57 +0100, Cezary Rojewski wrote:

...

>> +int avs_ipc_set_dx(struct avs_dev *adev, u32 core_mask, bool
>> powerup)
> a description for this function would be helpful.


Both relevant fields have comments as requested by Pierre.

We put quite an effort - which was triggered thanks to the feedback 
received - to have plain, simple delegates in form of: set necessary 
bits and let firmware to its job. I believe SET_DX is a good example of 
such.

>> +{
>> +	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);
>> +
>> +	ret = avs_dsp_send_msg(adev, &request, NULL);
>> +	if (ret)
>> +		avs_ipc_err(adev, &request, "set dx", ret);
>> +
>> +	return ret;
>> +}
>> +
>> +/*
>> + * avs_ipc_set_d0ix - Set power gating policy (entering D0IX
>> substates)
>> + *
>> + * @enable_pg: Whether to enable or disable power gating
>> + * @streaming: Whether a stream is running when transitioning
>> + */
> What do you mean why "whether a stream is running" does it mean any
> stream? What is the difference between a D0ix transition with a stream
> running compared to not running?


Precisely, any stream. The transition details are found within the 
firmware. Host (the driver) is responsible for scheduling and requesting 
the transition. Nothing more, nothing less.


Regards,
Czarek