hda: hw support for hda chain #6656

makarukp · 2022-11-23T13:52:29Z

This PR enables hardware support for HDA chain transfers (without deploying internal pipelines):
Content:

hda chain management
hda chain tasks and DMA logic implementation

Signed-off-by: Piotr Makaruk piotr.makaruk@intel.com

src/audio/pipeline/chain_dma.c

src/include/sof/audio/chain_dma.h

src/ipc/ipc4/helper.c

src/audio/pipeline/chain_dma.c

src/include/sof/audio/chain_dma.h

zephyr/CMakeLists.txt

src/audio/pipeline/chain_dma.c

src/audio/chain_dma.c

lgirdwood · 2022-11-30T13:42:00Z

@makarukp is this PR still pending validation or implementation completion or external dependencies ?

src/audio/chain_dma.c

makarukp · 2022-12-01T08:25:49Z

@lgirdwood This PR is still under development, it requires at lease one more commit with code completion before it is ready for validation.

makarukp · 2022-12-15T09:57:48Z

Code is completed. PR is ready for validation.

src/include/sof/audio/chain_dma.h

src/audio/chain_dma.c

lyakh · 2023-01-13T07:29:23Z

src/audio/chain_dma.c

yeah, this looks a bit suspicious. And in fact dw_dma_get_status() can return -EINVAL if the channel number is invalid, and that is a hard error and in that case stat isn't filled. So, this at least deserves a comment, much more so than the two comments above which explain something, that's pretty clear from the code itself.

src/audio/chain_dma.c

src/include/sof/audio/chain_dma.h

src/audio/chain_dma.c

src/ipc/ipc4/helper.c

src/ipc/ipc4/handler.c

makarukp · 2023-01-13T13:36:08Z

@lyakh What do you think about such approach of critical error handling in response to 'dma_get_status()' ?

/* Status can return -ENODATA and current status if xrun occurs, then it is not critical
 * and flow shall continue. In worst case, when channel index exceeds max value, status can
 * return -EINVAL, then it is critical error.
 */
ret = dma_get_status(cd->chan_link->dma->z_dev, cd->chan_link->index, &stat);
switch (ret) {
case 0:
	break;
case -ENODATA:
	tr_warn(&chain_dma_tr, "chain_task_run(): dma_get_status() xrun occured, ret = %u",
	ret);
	break;
default:
	tr_err(&chain_dma_tr, "chain_task_run(): dma_get_status() error, ret = %u", ret);
	return SOF_TASK_STATE_COMPLETED;
}

lyakh · 2023-01-16T06:56:20Z

Status can return -ENODATA and current status if xrun occurs

According to https://www.alsa-project.org/alsa-doc/alsa-lib/pcm.html -EPIPE is used by ALSA to indicate an xrun?

makarukp · 2023-01-16T08:47:35Z

@lyakh Accually -EINVAL can be returned in current implementation only by dw_dma_get_status() which is used by GPDMA in Zephyr. DMA uses intel_adsp_hda_dma_status() and there is only assertion __ASSERT(channel < cfg->dma_channels, "Channel does not exist"); .

kv2019i

This starts to look good. A few minor issues, but no showstoppers.

src/audio/chain_dma.c

src/ipc/ipc4/handler.c

app/boards/intel_adsp_cavs25_tgph.conf

src/audio/chain_dma.c

lyakh · 2023-01-17T07:05:58Z

src/ipc/ipc4/handler.c

ipc4_chain_manager_create() seems to be a better name for this, but it's already there

Hardware HDA chain require manager for controlling multiple streams tasks. Enable support with LL task and DMA mandatory logic. Signed-off-by: Piotr Makaruk <piotr.makaruk@intel.com>

HW chain dma support require new logic in handler Signed-off-by: Piotr Makaruk <piotr.makaruk@intel.com>

Enable building chain dma component on TGL platform Signed-off-by: Piotr Makaruk <piotr.makaruk@intel.com>

Enable building chain dma component on MTL platform Signed-off-by: Piotr Makaruk <piotr.makaruk@intel.com>

Enable building chain dma component on TGL-H platform Signed-off-by: Piotr Makaruk <piotr.makaruk@intel.com>

lgirdwood · 2023-01-18T13:38:13Z

src/audio/chain_dma.c

@lyakh this looks resolved now, error code is checked ?

lgirdwood · 2023-01-18T13:39:36Z

src/audio/chain_dma.c

@ujfalusi resolved ?

marc-hb · 2023-01-20T07:37:56Z

This apparently caused regressions #6975 and #6976.

It was also merged while failing a large number of tests: https://sof-ci.01.org/sofpr/PR6656/build3409/devicetest/index.html

lgirdwood · 2023-01-20T11:12:35Z

There is a lot of test failures on ADL capture, I've created a revert PR that can be merged today prior to daily test runs if we don't have fixes ready in time.

lgirdwood · 2023-01-20T11:13:37Z

@makarukp @mwasko @kv2019i @lyakh fyi - does anyone have fix PRs in progress otherwise we will have to revert ?

mwasko · 2023-01-20T11:32:59Z

@makarukp @mwasko @kv2019i @lyakh fyi - does anyone have fix PRs in progress otherwise we will have to revert ?

@lgirdwood, @kv2019i the regression is affecting only cAVS targets which are not POR in Zephyr configuration. The tests were passing before on cAVS because they were using w/a in form of copier-copier solution which is not correct implementation. We should treat it as we have enabled feature for MTL and for cAVS it is a to do action (on best effort basis).

lgirdwood · 2023-01-20T11:38:07Z

@makarukp @mwasko @kv2019i @lyakh fyi - does anyone have fix PRs in progress otherwise we will have to revert ?

@lgirdwood, @kv2019i the regression is affecting only cAVS targets which are not POR in Zephyr configuration. The tests were passing before on cAVS because they were using w/a in form of copier-copier solution which is not correct implementation. We should treat it as we have enabled feature for MTL and for cAVS it is a to do action (on best effort basis).

I'm fine to make this a MTL only feature, but it should not break legacy features on other platforms. Can this be fixed with a Kconfig ?

makarukp · 2023-01-20T12:20:36Z

@makarukp @mwasko @kv2019i @lyakh fyi - does anyone have fix PRs in progress otherwise we will have to revert ?

@lgirdwood, @kv2019i the regression is affecting only cAVS targets which are not POR in Zephyr configuration. The tests were passing before on cAVS because they were using w/a in form of copier-copier solution which is not correct implementation. We should treat it as we have enabled feature for MTL and for cAVS it is a to do action (on best effort basis).

I'm fine to make this a MTL only feature, but it should not break legacy features on other platforms. Can this be fixed with a Kconfig ?

@lgirdwood Unfortunately it is not possible to fix it with Kconfig because legacy approach was erased by hw chain dma PR.

As for #6975 (comment) issue it does not look like a big deal. I will prepare debug FW which could probably solve that problem or provide at least more information.

As for #6976 (comment) issue I don't know what happened and still thinking about further steps for debug ...

makarukp · 2023-01-23T11:35:10Z

I prepared fix for #6975:
#6983

makarukp requested a review from abonislawski November 23, 2022 14:06

paulstelian97 mentioned this pull request Nov 23, 2022

hw hda chain managment #6624

Closed

lyakh requested changes Nov 24, 2022

View reviewed changes

makarukp force-pushed the hw_hda_chain_support branch from 9be8324 to c7a1b11 Compare November 24, 2022 16:55

makarukp requested review from RanderWang, jsarha, libinyang, marcinszkudlinski and singalsu November 24, 2022 16:58

makarukp force-pushed the hw_hda_chain_support branch from c7a1b11 to 190506e Compare November 25, 2022 10:10

lgirdwood reviewed Nov 25, 2022

View reviewed changes

src/include/sof/audio/chain_dma.h Outdated Show resolved Hide resolved

zephyr/CMakeLists.txt Outdated Show resolved Hide resolved

src/audio/pipeline/chain_dma.c Outdated Show resolved Hide resolved

makarukp force-pushed the hw_hda_chain_support branch from 190506e to 081ddd5 Compare November 29, 2022 12:21

RanderWang reviewed Nov 30, 2022

View reviewed changes

src/audio/chain_dma.c Outdated Show resolved Hide resolved

RanderWang reviewed Nov 30, 2022

View reviewed changes

src/audio/chain_dma.c Outdated Show resolved Hide resolved

makarukp force-pushed the hw_hda_chain_support branch from a736998 to 4b4efff Compare November 30, 2022 08:39

marcinszkudlinski reviewed Nov 30, 2022

View reviewed changes

src/audio/chain_dma.c Outdated Show resolved Hide resolved

makarukp marked this pull request as ready for review December 14, 2022 14:24

makarukp requested review from aborisovich, dbaluta, lbetlej, marc-hb, mmaka1 and plbossart as code owners December 14, 2022 14:24

makarukp force-pushed the hw_hda_chain_support branch from 2b3c0fa to b508b85 Compare December 14, 2022 15:19

makarukp changed the title ~~[DO NOT MERGE]hda: hw support for hda chain~~ hda: hw support for hda chain Dec 15, 2022

makarukp requested review from pblaszko and softwarecki December 15, 2022 11:33

makarukp force-pushed the hw_hda_chain_support branch 2 times, most recently from 896317d to 6eb4ac3 Compare January 12, 2023 13:57

lyakh reviewed Jan 13, 2023

View reviewed changes

makarukp force-pushed the hw_hda_chain_support branch from 6eb4ac3 to 36e1c70 Compare January 16, 2023 11:48

kv2019i approved these changes Jan 16, 2023

View reviewed changes

src/audio/chain_dma.c Outdated Show resolved Hide resolved

src/ipc/ipc4/handler.c Outdated Show resolved Hide resolved

app/boards/intel_adsp_cavs25_tgph.conf Outdated Show resolved Hide resolved

makarukp force-pushed the hw_hda_chain_support branch from 36e1c70 to ca41ffa Compare January 16, 2023 12:38

lyakh reviewed Jan 17, 2023

View reviewed changes

makarukp added 5 commits January 17, 2023 09:06

hda: chain management and logic implementation

71b7363

Hardware HDA chain require manager for controlling multiple streams tasks. Enable support with LL task and DMA mandatory logic. Signed-off-by: Piotr Makaruk <piotr.makaruk@intel.com>

hda: enable handler for hw chain dma

dc1731b

HW chain dma support require new logic in handler Signed-off-by: Piotr Makaruk <piotr.makaruk@intel.com>

tgl: hda: enable chain dma

c966532

Enable building chain dma component on TGL platform Signed-off-by: Piotr Makaruk <piotr.makaruk@intel.com>

mtl: hda: enable chain dma

b91cab0

Enable building chain dma component on MTL platform Signed-off-by: Piotr Makaruk <piotr.makaruk@intel.com>

tgl-h: hda: enable chain dma

414a12c

Enable building chain dma component on TGL-H platform Signed-off-by: Piotr Makaruk <piotr.makaruk@intel.com>

makarukp force-pushed the hw_hda_chain_support branch from ca41ffa to 414a12c Compare January 17, 2023 08:13

lgirdwood approved these changes Jan 18, 2023

View reviewed changes

lyakh approved these changes Jan 18, 2023

View reviewed changes

mwasko approved these changes Jan 19, 2023

View reviewed changes

mwasko merged commit 507ad53 into thesofproject:main Jan 19, 2023

miRoox mentioned this pull request Jan 20, 2023

[BUG] FW reported error: 3018 - Unknown for 0xe070000|0x0 (GLB_CHAIN_DMA) when pause-release on TGLU_UP_HDA #6975

Closed

kv2019i mentioned this pull request Jan 20, 2023

[BUG] DSP is not suspended after playback is stopped 15s on TGLU_UP_HDA #6976

Closed

lgirdwood mentioned this pull request Jan 20, 2023

Revert "hda: hw support for hda chain" #6978

Closed

hda: hw support for hda chain #6656

hda: hw support for hda chain #6656

Uh oh!

Conversation

makarukp commented Nov 23, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lgirdwood commented Nov 30, 2022

Uh oh!

Uh oh!

makarukp commented Dec 1, 2022

Uh oh!

makarukp commented Dec 15, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lyakh Jan 13, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

makarukp commented Jan 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lyakh commented Jan 16, 2023

Uh oh!

makarukp commented Jan 16, 2023

Uh oh!

kv2019i left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lyakh Jan 17, 2023

Choose a reason for hiding this comment

Uh oh!

lgirdwood Jan 18, 2023

Choose a reason for hiding this comment

Uh oh!

lgirdwood Jan 18, 2023

Choose a reason for hiding this comment

Uh oh!

marc-hb commented Jan 20, 2023

Uh oh!

lgirdwood commented Jan 20, 2023

Uh oh!

lgirdwood commented Jan 20, 2023

Uh oh!

mwasko commented Jan 20, 2023

Uh oh!

lgirdwood commented Jan 20, 2023

Uh oh!

makarukp commented Jan 20, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

makarukp commented Nov 23, 2022 •

edited

Loading

makarukp commented Jan 13, 2023 •

edited

Loading

makarukp commented Jan 20, 2023 •

edited

Loading