dma: disable hda when pipeline is reset

[fkwasowi]

DGCS_GEN bit should be set to 0 on a pipeline state change to reset

[kv2019i]

The description is a bit misleading as it assume this code is always used with HD-DMA, while this code is in fact generic and could be used with a wide variety of DMA hardware and drivers.

But this does explain why this is important step to do (and explains what e.g. happens on HD-DMA hardware when not done), so not a blocker.

[ranj063]

@fkwasowi what issue does this change fix? AFAICT we also do dma_stop() in host_zephyr_trigger() in the STOP case with the same check for state. While the state may not be active during reset, i'd assume that it must have been active during stop and the dma should be stopped there

[fkwasowi]

There is a dma_start in host_zephyr_trigger that sets DGCS_GEN to 1, and a dma_stop that sets it to 0. Pipeline reset usually occurs after PAUSE (there is also a COMP_STATE_PREPARE state in between, and it is on this state that we enter the host_zephyr_reset function). I have not seen such a scenario that the RESET pipeline state is executed immediately after the RUN pipeline state (COMP_STATE_ACTIVE is only when we are in the RUN state). Such a sequence is incorrect. This fixes the scenario: https://github.com/intel-innersource/drivers.audio.firmware.converged/issues/505

[lyakh]

does this mean, that we're effectively reverting #6849 ? Was it wrong?

[plbossart]

I am also not following the change nor the failing scenario.

We do clear DGCS_GEN in hda_hda_stop_common, what causes it not to be cleared?

I'll add a "request change" to mark my concern, this DGCS_GEN bit is very very touchy and we have to be super careful with any changes to these sequences.

[fkwasowi]

IF was added because in sof the same channel was stopped several times in some scenarios. Because of this, the ref count in the dev pm was going off. But the problem was fixed later on the hda driver side, so now the IF can be removed.

[fkwasowi]

HDA DMA programming sequence in pipeline flows, coming from closed-source FW (obligatory to work properly with closed-source driver):

Reset (XRUN):

IPC: Set Pipe State(PAUSE): FW: if coming from RUN state, then no DMA programming
SW: Set DMA RUN = 0 and Poll untill it reflects 0
IPC: Set Pipe State(RESET): FW:
a. GEN=0
b. FIFORDY=0
SW: HDA Stream RESET
SW: HDA Stream Configure(BDL, CTL etc.)
SW: start Host DMA
IPC: Set Pipe State(PAUSE): FW: if coming from RESET state then FIFORDY=1
IPC: Set Pipe State (RUN): FW: Set GEN=1

[plbossart]

IF was added because in sof the same channel was stopped several times in some scenarios. Because of this, the ref count in the dev pm was going off. But the problem was fixed later on the hda driver side, so now the IF can be removed.

That's a valid explanation @fkwasowi, however why not revert fa0456e completely then?

We still have cases where the dma_stop is handled conditionally, e.g.

case COMP_TRIGGER_STOP:
	case COMP_TRIGGER_XRUN:
		if (dev->state == COMP_STATE_ACTIVE) {
			ret = dma_stop(hd->chan->dma->z_dev, hd->chan->index);
			if (ret < 0)
				comp_err(dev, "host_trigger(): dma stop failed: %d",
					 ret);
		}

[fkwasowi]

@tmleman Is it necessary, to undo all your changes fa0456e ?

[tmleman]

Is it necessary, to undo all your changes fa0456e ?

I don't know if it's necessary but it should be possible to do. This has already been partially done in this commit
a942f10

[plbossart]

I am also not following the change nor the failing scenario.

We do clear DGCS_GEN in hda_hda_stop_common, what causes it not to be cleared?

I'll add a "request change" to mark my concern, this DGCS_GEN bit is very very touchy and we have to be super careful with any changes to these sequences.

[kv2019i]

@fkwasowi I see you asked for re-review from @plbossart . Can you confirm his concern was addressed (w.r.t. "what causes it not to be cleared?"..?

[fkwasowi]

@fkwasowi I see you asked for re-review from @plbossart . Can you confirm his concern was addressed (w.r.t. "what causes it not to be cleared?"..?

In the reported scenario, we did not enter the hda_dma_stop_common function.

[fkwasowi]

SOFCI TEST