Re: [PATCH 08/14] ASoC: Intel: avs: D0ix power state support

[Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>]

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.

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.

>  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?

> +		 * 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. */

??

> +		if (ret == -AVS_EIPC)
> +			atomic_inc(&ipc->d0ix_disable_depth);
> +
> +		ipc->in_d0ix = false;
> +		return ret;
> +	}
> +
> +	ipc->in_d0ix = enable;
> +	return 0;
> +}
> +
> +static void avs_dsp_schedule_d0ix(struct avs_dev *adev, struct avs_ipc_msg *tx)
> +{
> +	if (atomic_read(&adev->ipc->d0ix_disable_depth))
> +		return;
> +
> +	mod_delayed_work(system_power_efficient_wq, &adev->ipc->d0ix_work,
> +			 msecs_to_jiffies(AVS_D0IX_DELAY_MS));
> +}
> +
> +static void avs_dsp_d0ix_work(struct work_struct *work)
> +{
> +	struct avs_ipc *ipc = container_of(work, struct avs_ipc, d0ix_work.work);
> +
> +	avs_dsp_set_d0ix(to_avs_dev(ipc->dev), true);
> +}
> +
> +static int avs_dsp_wake_d0i0(struct avs_dev *adev, struct avs_ipc_msg *tx)
> +{
> +	struct avs_ipc *ipc = adev->ipc;
> +
> +	if (!atomic_read(&ipc->d0ix_disable_depth)) {
> +		cancel_delayed_work_sync(&ipc->d0ix_work);
> +		return avs_dsp_set_d0ix(adev, false);
> +	}
> +
> +	return 0;
> +}
> +
> +int avs_dsp_disable_d0ix(struct avs_dev *adev)
> +{
> +	struct avs_ipc *ipc = adev->ipc;
> +
> +	/* Prevent PG only on the first disable. */
> +	if (atomic_add_return(1, &ipc->d0ix_disable_depth) == 1) {
> +		cancel_delayed_work_sync(&ipc->d0ix_work);
> +		return avs_dsp_set_d0ix(adev, false);
> +	}
> +
> +	return 0;
> +}
> +
> +int avs_dsp_enable_d0ix(struct avs_dev *adev)
> +{
> +	struct avs_ipc *ipc = adev->ipc;
> +
> +	if (atomic_dec_and_test(&ipc->d0ix_disable_depth))
> +		queue_delayed_work(system_power_efficient_wq, &ipc->d0ix_work,
> +				   msecs_to_jiffies(AVS_D0IX_DELAY_MS));
> +	return 0;
> +}
>  
>  static void avs_dsp_recovery(struct avs_dev *adev)
>  {
> @@ -391,10 +467,35 @@ static int avs_dsp_do_send_msg(struct avs_dev *adev, struct avs_ipc_msg *request
>  	return ret;
>  }
>  
> +static int avs_dsp_send_msg_sequence(struct avs_dev *adev, struct avs_ipc_msg *request,
> +				     struct avs_ipc_msg *reply, int timeout, bool wake_d0i0,
> +				     bool schedule_d0ix)
> +{
> +	int ret;
> +
> +	if (wake_d0i0) {
> +		ret = avs_dsp_wake_d0i0(adev, request);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	ret = avs_dsp_do_send_msg(adev, request, reply, timeout);
> +	if (ret)
> +		return ret;
> +
> +	if (schedule_d0ix)
> +		avs_dsp_schedule_d0ix(adev, request);
> +
> +	return 0;
> +}
> +
>  int avs_dsp_send_msg_timeout(struct avs_dev *adev, struct avs_ipc_msg *request,
>  			     struct avs_ipc_msg *reply, int timeout)
>  {
> -	return avs_dsp_do_send_msg(adev, request, reply, timeout);
> +	bool wake_d0i0 = avs_dsp_op(adev, d0ix_toggle, request, true);
> +	bool schedule_d0ix = avs_dsp_op(adev, d0ix_toggle, request, false);
> +
> +	return avs_dsp_send_msg_sequence(adev, request, reply, timeout, wake_d0i0, schedule_d0ix);
>  }
>  
>  int avs_dsp_send_msg(struct avs_dev *adev, struct avs_ipc_msg *request,
> @@ -403,6 +504,19 @@ int avs_dsp_send_msg(struct avs_dev *adev, struct avs_ipc_msg *request,
>  	return avs_dsp_send_msg_timeout(adev, request, reply, adev->ipc->default_timeout_ms);
>  }
>  
> +int avs_dsp_send_pm_msg_timeout(struct avs_dev *adev, struct avs_ipc_msg *request,
> +				struct avs_ipc_msg *reply, int timeout, bool wake_d0i0)
> +{
> +	return avs_dsp_send_msg_sequence(adev, request, reply, timeout, wake_d0i0, false);
> +}
> +
> +int avs_dsp_send_pm_msg(struct avs_dev *adev, struct avs_ipc_msg *request,
> +			struct avs_ipc_msg *reply, bool wake_d0i0)
> +{
> +	return avs_dsp_send_pm_msg_timeout(adev, request, reply, adev->ipc->default_timeout_ms,
> +					   wake_d0i0);
> +}
> +
>  static int avs_dsp_do_send_rom_msg(struct avs_dev *adev, struct avs_ipc_msg *request, int timeout)
>  {
>  	struct avs_ipc *ipc = adev->ipc;
> @@ -463,6 +577,7 @@ int avs_ipc_init(struct avs_ipc *ipc, struct device *dev)
>  	ipc->ready = false;
>  	ipc->default_timeout_ms = AVS_IPC_TIMEOUT_MS;
>  	INIT_WORK(&ipc->recovery_work, avs_dsp_recovery_work);
> +	INIT_DELAYED_WORK(&ipc->d0ix_work, avs_dsp_d0ix_work);
>  	init_completion(&ipc->done_completion);
>  	init_completion(&ipc->busy_completion);
>  	spin_lock_init(&ipc->rx_lock);
> @@ -475,4 +590,6 @@ void avs_ipc_block(struct avs_ipc *ipc)
>  {
>  	ipc->ready = false;
>  	cancel_work_sync(&ipc->recovery_work);
> +	cancel_delayed_work_sync(&ipc->d0ix_work);
> +	ipc->in_d0ix = false;
>  }
> diff --git a/sound/soc/intel/avs/messages.c b/sound/soc/intel/avs/messages.c
> index 3da33150aabf..6404fce8cde4 100644
> --- a/sound/soc/intel/avs/messages.c
> +++ b/sound/soc/intel/avs/messages.c
> @@ -432,7 +432,7 @@ int avs_ipc_set_dx(struct avs_dev *adev, u32 core_mask, bool powerup)
>  	request.data = &dx;
>  	request.size = sizeof(dx);
>  
> -	ret = avs_dsp_send_msg(adev, &request, NULL);
> +	ret = avs_dsp_send_pm_msg(adev, &request, NULL, true);
>  	if (ret)
>  		avs_ipc_err(adev, &request, "set dx", ret);
>  
> @@ -456,7 +456,7 @@ int avs_ipc_set_d0ix(struct avs_dev *adev, bool enable_pg, bool streaming)
>  
>  	request.header = msg.val;
>  
> -	ret = avs_dsp_send_msg(adev, &request, NULL);
> +	ret = avs_dsp_send_pm_msg(adev, &request, NULL, false);
>  	if (ret)
>  		avs_ipc_err(adev, &request, "set d0ix", ret);
>