ASoC: SOF: (Intel HDA) Add support for DSPless debug mode #3962

ujfalusi · 2022-10-26T08:43:23Z

Replacing #3958, now with the correct branch!

Hi,

this series will enable the use of the SOF Linux stack without DSP offloading.

In this mode no firmware loading will happen, the topology parsing is going to be reduced to only look for the DAI widget(s) and the IPC dependent callbacks are going to be ignored.

This mode can give another level of hardware verification on platforms where the DSP can be ignored and the audio interfaces can be tested directly.

On Intel platforms we can use this mode to verify programming flows against the legacy HDA stack if there is a need to debug in that level and don't have the DSP programming sequences interfering.

The use of DSPless mode is governed by the SOF_DBG_DSPLESS_MODE flag which is only going to be set if the user sets sof_debg=0x8000 and the platform advertises that the DSPless mode is supported on them.

ujfalusi · 2022-10-26T08:46:02Z

Changes since v1:

Use sdev->dspless_mode_selected runtime for decisions instead of the sof_debug flag
all minor comments addressed with the exception of the use of a temporary tplg pointer

ujfalusi · 2022-10-26T11:24:32Z

Changes since v2:

set constraint on the supported formats for HDA in DSPless mode: Only S16 and S32 formats are supported (S24_LE is not supported).

sound/soc/sof/intel/hda-dai.c

ujfalusi · 2022-10-27T11:18:58Z

Changes since v3:

do not allow DSPless mode to be enabled if the NOCODEC mode is selected
Do not connect non HDA DAIs to allow mixed topology loading but make sure that the PCMs with non HDA DAIs are not usable by user.
Use the correct CONFIG_SND_SOC_SOF_HDA_AUDIO_CODEC in hda-dai.c

plbossart · 2022-10-27T12:51:52Z

do not allow DSPless mode to be enabled if the NOCODEC mode is selected

This needs to be platform-dependent @ujfalusi. If we hard-code this we are going to un-hard-code it soon.

ujfalusi · 2022-10-28T12:31:30Z

Changes since v4:

Lift additional restriction in core on the DSPless mode (NOCODEC and INTEL_HDA dai)
Introduce hda_get_interface_mask() in intel/hda.c to get an interface mask about the currently supported interfaces
the mask is Intel audio IP and DSPless mode dependent.
Only do discovery of I2S, DMIC and SDW if they are supported.

The only issue with the implementation is that if the DSPless mode is selected and also the NOCODEC is forced we will end up with a card which is not usable as nocodec topologies usually falling back to use SSP and DMIC and they are not usable without DSP.

Anyways, this PR works OK on UP2 when using the pcm512x ACPI patching and the resulting card will fall back to a generic HDA card:

[12958.191985] sof-audio-pci-intel-apl 0000:00:0e.0: Switching to DSPless mode
...
[12958.249559] snd_sof:sof_machine_register: sof-audio-pci-intel-apl 0000:00:0e.0: created machine skl_hda_dsp_generic
[12958.249580] snd_sof_pci:sof_pci_probe_complete: sof-audio-pci-intel-apl 0000:00:0e.0: Completing SOF PCI probe
[12958.267497] snd_sof:snd_sof_load_topology: sof-audio-pci-intel-apl 0000:00:0e.0: loading topology:intel/sof-tplg/sof-hda-generic-idisp.tplg

and

# aplay -l
**** List of PLAYBACK Hardware Devices ****
card 0: sofhdadsp [sof-hda-dsp], device 1: HDMI1 (*) []
  Subdevices: 1/1
  Subdevice #0: subdevice #0
card 0: sofhdadsp [sof-hda-dsp], device 2: HDMI2 (*) []
  Subdevices: 1/1
  Subdevice #0: subdevice #0
card 0: sofhdadsp [sof-hda-dsp], device 3: HDMI3 (*) []
  Subdevices: 1/1
  Subdevice #0: subdevice #0

sound/soc/sof/intel/hda-dai.c

plbossart

@ujfalusi we're going to need some more time to review the last patches, so can we push the initial 13-odd patches in a separate PR to reduce the volume of patches to carry in this draft?

more comments below.

sound/soc/sof/intel/hda-dai.c

plbossart · 2022-10-31T22:08:38Z

sound/soc/sof/core.c

remove error: prefix in all this code?

ujfalusi · 2022-11-02T11:46:24Z

@ujfalusi we're going to need some more time to review the last patches, so can we push the initial 13-odd patches in a separate PR to reduce the volume of patches to carry in this draft?

I have separated the ops optionality patches s they are one topic (but in itself it does not have much reason to exist): #3986

plbossart · 2022-11-07T17:06:56Z

@ujfalusi can you rebase so that we look at the latest and greatest code on this branch?

ujfalusi · 2022-11-08T08:05:06Z

Changes since v5:

rebased
Separated the skip unsupported interfaces from the DSPless mode patch to make it easier to focus on the real changes.

plbossart · 2022-11-17T19:39:24Z

@ujfalusi would you be able to rebase this PR on top of #/3972? It'd be good if @ranj063 and I can take a look tomorrow morning (Nov 18) before the week-end and the Turkey break. Thanks!

plbossart · 2022-11-18T00:37:57Z

#3972 is merged, please rebase @ujfalusi

ujfalusi · 2022-11-18T12:30:48Z

Changes since v6:

reworked on top of sof-dev with Abstract the DMA and Codec DAI ops for the BE DAI widgets #3972 merged
Things might need to be revisited since Abstract the DMA and Codec DAI ops for the BE DAI widgets #3972 made IPC mode semi mandatory for the HDA code.
If there is no swidget allocated, it implies that no IPC is used, which is the case of the DSPless mode

ujfalusi · 2022-11-18T14:29:36Z

Oops, something is not quite how it should be in DSP mode...

ujfalusi · 2022-11-18T14:45:37Z

Changes since v7:

DSP mode fixed (broke due to returning uninitialized 'ops' from hda_dai_get_ops() on the second call)
dropped the 'error:' prefix from a print in core

plbossart

Couple of comments @ujfalusi , I wasn't able to follow some of the changes on why we need a setup function and other options. Nitpicks otherwise.

plbossart · 2022-11-18T14:59:01Z

sound/soc/sof/pm.c

@ranj063 has a different solution for the same problem it seems @ujfalusi

see 297cb2f

which one should we take?

They are identical in function, we can take @ranj063's version if that makes in earlier (I'm sure it will)

this patch needs to go now that my PR is merged right @ujfalusi ?

indeed this seems to be the source of conflicts?

sound/soc/sof/sof-audio.c

sound/soc/sof/intel/hda-dai-ops.c

plbossart · 2022-11-18T15:10:27Z

sound/soc/sof/intel/hda-dai.c

I was not able to follow the change from the initial code written by @ranj063

In one case sdai->platform_private is required and not in the other.

there's no swidget and so there's no sdai with dspless mode @plbossart. so there's cant be a platform_private either

This change might not be needed if the swidget allocation way works.

plbossart · 2022-11-18T15:11:08Z

sound/soc/sof/intel/hda-dai.c

why is setup_hext_stream mandatory now?

I don't think we can have different set of mandatory ops for different instances of the ops. setup() is optional for this exact reason.

This is exactly what I was saying for the Abstraction PR. This two op is mandatory for normal mode and for DSPless.
In fact, the hext_stream ops are mandatory for the normal mode.
I can drop the mandatory check, but it is mandatory.

And the fact is that different modes (normal chain and DSPless) very well needs different sets of mandatory callbacks.

@ujfalusi different modes needing different ops doesnt make the ops mandatory. Its a bit like saying the IPC ops we have for IPC3/IPC4 will need to have different set of mandatory ops right? But if you think setup is mandatory for all cases, lets make it mandatory

plbossart · 2022-11-18T15:13:11Z

sound/soc/sof/intel/hda-stream.c

actually the initial code is rotten already. We assign hstream by dereferencing hext_stream BEFORE testing hext_stream. This is not right. both hstream and mask assignments need to be placed after the test for if (!hext_stream)

Hah, that's true. I'll fix that as well with a separate patch.

plbossart · 2022-11-18T15:14:05Z

sound/soc/sof/intel/hda-stream.c

BIT(hsteam->index) for consistency?

sound/soc/sof/intel/hda.c

sound/soc/sof/intel/pci-apl.c

ranj063 · 2022-11-18T16:41:23Z

sound/soc/sof/core.c

can we combine these 2 inot 1 condition with && and the else can have a single info saying "DSPless mode sectiond ignored"? That will reduce the indendation a bit

It is a good information that the DSPless mode is not selected due to platform not supporting it and the user request is ignored.

it is not the same as the user has not asked for it in the first place.

ranj063 · 2022-11-18T16:41:48Z

sound/soc/sof/sof-priv.h

sound/soc/sof/core.c

ranj063 · 2022-11-18T16:53:18Z

sound/soc/sof/intel/hda-dai.c

I don't think we can have different set of mandatory ops for different instances of the ops. setup() is optional for this exact reason.

ranj063 · 2022-11-18T16:55:13Z

sound/soc/sof/intel/hda-dai.c

there's no swidget and so there's no sdai with dspless mode @plbossart. so there's cant be a platform_private either

ranj063 · 2022-11-18T17:01:26Z

sound/soc/sof/intel/hda-dai.c

@ujfalusi I think something along these lines as below might look better. What do you think?

static const struct hda_dai_widget_dma_ops * hda_dai_get_ops(struct snd_pcm_substream *substream, struct snd_soc_dai *cpu_dai) { struct snd_soc_dapm_widget *w = snd_soc_dai_get_widget(cpu_dai, substream->stream); struct snd_sof_dev *sdev = snd_soc_component_get_drvdata(cpu_dai->component); struct snd_sof_widget *swidget = w->dobj.private; const struct hda_dai_widget_dma_ops *ops; struct snd_sof_dai *sdai; /* return already set ops */ if (swidget) { sdai = swidget->private; if (sdai->platform_private) return sdai->platform_private; } /* select the op */ ops = hda_select_dai_widget_ops(sdev, swidget); if (!ops) return NULL; /* check if mandatory ops are set */ if (!ops || !ops->get_hext_stream) return NULL; /* set the ops for DSP mode */ if (swidget) sdai->platform_private = ops; return ops; }

@ujfalusi I also have an alternate suggestion. We have a the snd_soc_dapm_widget fr the BE DAI, what we're missing is swidget. Now, if we could modify the topology ops like this, you wont have to touch this function or hda_dai_config() for that matter

static int sof_dspless_widget_ready(struct snd_soc_component *scomp, int index, struct snd_soc_dapm_widget *w, struct snd_soc_tplg_dapm_widget *tw) { struct snd_sof_dev *sdev = snd_soc_component_get_drvdata(scomp); swidget = kzalloc(sizeof(*swidget), GFP_KERNEL); if (!swidget) return -ENOMEM; switch (w->id) { case snd_soc_dapm_dai_in: case snd_soc_dapm_dai_out: struct snd_sof_dai *dai; dai = kzalloc(sizeof(*dai), GFP_KERNEL); if(!dai) return -ENOMEM; ret = sof_connect_dai_widget(scomp, w, tw, dai); if (ret < 0) { kfree(swidget); kfree(dai); return ret; } swidget->private = dai; break; default: break; } w->dobj.private = swidget; return 0 } static int sof_dspless_widget_unload(struct snd_soc_component *scomp, struct snd_soc_dobj *dobj) { struct snd_sof_widget *swidget = dobj->private; if (!swidget) return 0; kfree(swidget->private); kfree(swidget); }

I have thought about the second way to allocate the swidget, let me test on Monday if it brings other issues or not.

Right, so this is not going to work without doing a full blown topology parsing in case of DSPless mode as we need to set up all the things to get the information (ipc4_copier->dai_type or dai_config->type)
I think I will replace the !swidget checks with explicit dspless_mode_selected queries to not obscure the code.
Yep, less generic, but easier to understand.

sound/soc/sof/topology.c

sound/soc/sof/intel/hda.c

ujfalusi · 2022-12-01T14:52:47Z

Changes since v10:

Update the correct comment about the nonatomic flag
Fix the typo in the comment for skipping sndw if it is not available on the platform.

ranj063

looks great. thanks @ujfalusi

plbossart

only one open with refcounts, looks good otherwise. Thanks @ujfalusi

sound/soc/sof/sof-audio.c

…safe Only access hext_stream->hstream after it has been checked for NULL pointer Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>

Skip preparing/unpreparing widgets if the swidget pointer is NULL. This will be true in the case of virtual widgets in topology that were added for reusing the legacy HDA machine driver with SOF. Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>

The DSPless mode of the ASoC/SOF driver can be used for hardware verification and debug on platforms with HDaudio codecs. The DSP mode is still needed on existing platforms for SSP, DMIC, SoundWire interfaces managed by the GP-DMA. This mode is also helpful to compare the legacy HDaudio driver with the ASoC/SOF driver wrt. codec management and handling. In theory we use the same code but differences are sometimes seen on jack detection and event handling. Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>

Via the SOF_DBG_DSPLESS_MODE sof_debug flag the SOF stack can be asked to not use the DSP for audio. The core's support for DSPless mode is only going to be enabled if the platform reports that it can be used without DSP. Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>

Not all interfaces (SSP/DMIC/HDA/SDW) are available on all platforms. If the interface is not even supported then there is no point in executing a probe or query for that interface. Introduce a simple function (hda_get_interface_mask) to query the interfaces supported on the platform. Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>

Via the SOF_DBG_DSPLESS_MODE sof_debug flag the SOF stack can be asked to not use the DSP for audio. The use of DSPless mode is governed by the sdev->dspless_mode_selected flag which is only going to be set if the user sets sof_debug=0x8000 and the platform advertises that the DSPless mode is supported on them. Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>

set the dspless_mode_supported flag to true for apl family to allow DSPless mode. Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>

set the dspless_mode_supported flag to true for cnl/cfl/cml family to allow DSPless mode. Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>

set the dspless_mode_supported flag to true for icl/jsl family to allow DSPless mode. Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>

set the dspless_mode_supported flag to true for mtl family to allow DSPless mode. Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>

set the dspless_mode_supported flag to true for skl family to allow DSPless mode. Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>

set the dspless_mode_supported flag to true for tgl/adl family to allow DSPless mode. Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>

ujfalusi · 2022-12-01T19:53:29Z

Changes since v11:

correct the use_count test which was carried over from an old revision (in patch 2).

plbossart

Thanks @ujfalusi, nice work!

RanderWang

LGTM !

ujfalusi requested a review from jsarha October 26, 2022 08:43

ujfalusi force-pushed the peter/sof/pr/dspless_mode_01 branch from 076fa16 to 28544b0 Compare October 26, 2022 11:22

plbossart reviewed Oct 26, 2022

View reviewed changes

sound/soc/sof/intel/hda-dai.c Outdated Show resolved Hide resolved

ujfalusi force-pushed the peter/sof/pr/dspless_mode_01 branch from 28544b0 to 9592dd9 Compare October 27, 2022 11:16

ujfalusi force-pushed the peter/sof/pr/dspless_mode_01 branch from 9592dd9 to 9a505dd Compare October 28, 2022 12:32

ranj063 reviewed Oct 28, 2022

View reviewed changes

sound/soc/sof/intel/hda-dai.c Outdated Show resolved Hide resolved

plbossart reviewed Oct 31, 2022

View reviewed changes

plbossart mentioned this pull request Nov 3, 2022

ASoC: SOF: Don't read reply if its not used #3925

Merged

ujfalusi force-pushed the peter/sof/pr/dspless_mode_01 branch from 9a505dd to 7c19f35 Compare November 8, 2022 08:02

ujfalusi force-pushed the peter/sof/pr/dspless_mode_01 branch from 7c19f35 to a8ef082 Compare November 18, 2022 12:28

ujfalusi force-pushed the peter/sof/pr/dspless_mode_01 branch 3 times, most recently from 0057093 to c88c5dc Compare November 18, 2022 14:43

plbossart reviewed Nov 18, 2022

View reviewed changes

ranj063 reviewed Nov 18, 2022

View reviewed changes

ujfalusi force-pushed the peter/sof/pr/dspless_mode_01 branch from c88c5dc to d0b33d0 Compare November 21, 2022 14:45

ujfalusi requested review from ajitkupandey, kuanhsuncheng, kv2019i and yaochunhung as code owners December 1, 2022 14:23

ranj063 reviewed Dec 1, 2022

View reviewed changes

sound/soc/sof/topology.c Outdated Show resolved Hide resolved

ranj063 reviewed Dec 1, 2022

View reviewed changes

sound/soc/sof/intel/hda.c Outdated Show resolved Hide resolved

ujfalusi force-pushed the peter/sof/pr/dspless_mode_01 branch from 3f2d8d6 to c15561c Compare December 1, 2022 14:52

ranj063 previously approved these changes Dec 1, 2022

View reviewed changes

plbossart requested changes Dec 1, 2022

View reviewed changes

sound/soc/sof/sof-audio.c Outdated Show resolved Hide resolved

ujfalusi and others added 12 commits December 1, 2022 21:52

ASoC: SOF: Intel: hda-stream: Do not dereference hstream until it is …

90c088b

…safe Only access hext_stream->hstream after it has been checked for NULL pointer Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>

ASoC: SOF: Intel: pci-apl: Allow DSPless mode

d3a07ae

set the dspless_mode_supported flag to true for apl family to allow DSPless mode. Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>

ASoC: SOF: Intel: pci-cnl: Allow DSPless mode

16a20b8

set the dspless_mode_supported flag to true for cnl/cfl/cml family to allow DSPless mode. Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>

ASoC: SOF: Intel: pci-icl: Allow DSPless mode

b025a8c

set the dspless_mode_supported flag to true for icl/jsl family to allow DSPless mode. Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>

ASoC: SOF: Intel: pci-mtl: Allow DSPless mode

1391992

set the dspless_mode_supported flag to true for mtl family to allow DSPless mode. Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>

ASoC: SOF: Intel: pci-skl: Allow DSPless mode

f35598a

set the dspless_mode_supported flag to true for skl family to allow DSPless mode. Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>

ASoC: SOF: Intel: pci-tgl: Allow DSPless mode

4936fa6

set the dspless_mode_supported flag to true for tgl/adl family to allow DSPless mode. Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>

ujfalusi dismissed ranj063’s stale review via 4936fa6 December 1, 2022 19:52

ujfalusi force-pushed the peter/sof/pr/dspless_mode_01 branch from c15561c to 4936fa6 Compare December 1, 2022 19:52

ranj063 approved these changes Dec 1, 2022

View reviewed changes

plbossart approved these changes Dec 1, 2022

View reviewed changes

RanderWang approved these changes Dec 2, 2022

View reviewed changes

plbossart merged commit e1e687b into thesofproject:topic/sof-dev Dec 2, 2022

ujfalusi deleted the peter/sof/pr/dspless_mode_01 branch February 21, 2023 08:42

ASoC: SOF: (Intel HDA) Add support for DSPless debug mode #3962

ASoC: SOF: (Intel HDA) Add support for DSPless debug mode #3962

Uh oh!

Conversation

ujfalusi commented Oct 26, 2022

Uh oh!

ujfalusi commented Oct 26, 2022

Uh oh!

ujfalusi commented Oct 26, 2022

Uh oh!

Uh oh!

ujfalusi commented Oct 27, 2022

Uh oh!

plbossart commented Oct 27, 2022

Uh oh!

ujfalusi commented Oct 28, 2022

Uh oh!

Uh oh!

plbossart left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ujfalusi commented Nov 2, 2022

Uh oh!

plbossart commented Nov 7, 2022

Uh oh!

ujfalusi commented Nov 8, 2022

Uh oh!

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

Uh oh!

Uh oh!

plbossart commented Nov 18, 2022

Uh oh!

ujfalusi commented Nov 18, 2022

Uh oh!

ujfalusi commented Nov 18, 2022

Uh oh!

ujfalusi commented Nov 18, 2022

Uh oh!

plbossart left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

plbossart commented Nov 17, 2022 •

edited

Loading

ranj063 Nov 28, 2022 •

edited

Loading

ranj063 Nov 18, 2022 •

edited

Loading