ASoC: SOF: (Intel HDA) Add support for DSPless debug mode #3958
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ujfalusi
force-pushed
the
peter/sof/ipc4_work_wip
branch
from
6b188d7 to
fdb9615
Compare
Collaborator
Author
|
Changes since v1:
|
Collaborator
Author
|
@jsarha, fyi |
ujfalusi
force-pushed
the
peter/sof/ipc4_work_wip
branch
2 times, most recently
from
9923c1f to
e51a3e2
Compare
Member
|
@ujfalusi can we remove the WIP patches? |
plbossart
reviewed
Oct 26, 2022
sound/soc/sof/sof-audio.c
Outdated
Member
There was a problem hiding this comment.
typo in commit title: prepare
sound/soc/sof/core.c
Outdated
Member
There was a problem hiding this comment.
nit-pick: would the code not be more readable with a boolean flag, e.g. sdev->dspless_selected. I see many many repeats of this 30-odd string, it's a bit of an eyesore.
Collaborator
Author
There was a problem hiding this comment.
I did not wanted to introduce a new boolean in sdev, but the clearing of the bit in sof_core_debug might not be a nice thing?
Let me check if there is anything preventing this.
sound/soc/sof/pcm.c
Outdated
Member
There was a problem hiding this comment.
maybe add a comment here to explain why we don't have a fixup?
sound/soc/sof/topology.c
Outdated
Member
There was a problem hiding this comment.
nit-pick: use a temp pointer and have a single call to tplg_component_load?
Collaborator
Author
There was a problem hiding this comment.
I think this is clear enough, I don's see the benefit in this particular case.
sound/soc/sof/intel/hda-dai.c
Outdated
Member
There was a problem hiding this comment.
commit message typo:
sof_debug=0x8000
not sure what "supported (tested)" means?
did you mean "the platform
advertises its ability to transfer audio without the DSP enabled" ?
sound/soc/sof/intel/hda-dai.c
Outdated
Member
There was a problem hiding this comment.
remove these empty-brackets or add a comment?
Collaborator
Author
There was a problem hiding this comment.
Oh, remained from debugging.
sound/soc/sof/intel/hda-dai.c
Outdated
Member
There was a problem hiding this comment.
do we need this condition? link_prepared is always set unconditionally
sound/soc/sof/intel/pci-apl.c
Outdated
Member
There was a problem hiding this comment.
commit title:spurious initial space
Collaborator
Author
There was a problem hiding this comment.
OK, let me check it
RanderWang
reviewed
Oct 26, 2022
sound/soc/sof/intel/hda-stream.c
Outdated
Collaborator
Author
There was a problem hiding this comment.
it is a code move, but sure, let's do that.
RanderWang
reviewed
Oct 26, 2022
sound/soc/sof/intel/hda.c
Outdated
There was a problem hiding this comment.
without this if, hdev->no_ipc_position would be 0 ?
Collaborator
Author
There was a problem hiding this comment.
Probably some shared code would try to send IPC? But in any case, since we don't use DSP the ipc_position is not even a possibility.
RanderWang
reviewed
Oct 26, 2022
sound/soc/sof/intel/pci-tgl.c
Outdated
There was a problem hiding this comment.
in dspless mode, which topology will be used ?
Collaborator
Author
There was a problem hiding this comment.
Depending on the ipc_type, works with both IPC3 or IPC4 and there should not be any IPC dependency
Collaborator
Author
There was a problem hiding this comment.
But yes, we are loading the topology file to get the DAIs for the PCMs.
Collaborator
Author
I have not removed them?? |
ujfalusi
force-pushed
the
peter/sof/ipc4_work_wip
branch
from
e51a3e2 to
3421206
Compare
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
This reverts commit 343f1d36f8925206709884a4bd22fa0dabf31d5a.
The following definitions have no users: SOF_DAI_STREAM() and SOF_FORMATS, they can be dropped from the header file. Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
The struct nhlt_endpoint is a flexible array, because:
struct nhlt_acpi_table {
...
struct nhlt_endpoint desc[] { <- This
...
struct nhlt_specific_cfg {
u32 size;
u8 caps[]; <- Because of this
};
};
};
This causes several notes from sparse as well:
note: in included file:
include/sound/intel-nhlt.h:83:34: warning: array of flexible structures
To help the compiler to understand this, we should convert it to u8 array.
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
The struct nhlt_fmt_cfg is a flexible array, because:
struct nhlt_fmt {
...
struct nhlt_fmt_cfg fmt_config[] { <- This
...
struct nhlt_specific_cfg {
...
u8 caps[]; <- Because of this
};
};
};
Strangely sparse did not complain about this, but it was misused by
drivers, treating it as a normal array and not a flexible one.
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
The items array member of truct sof_manifest is a flexible array, because:
struct sof_manifest {
...
struct sof_manifest_tlv items[] { <- This
...
__u8 data[]; << Because of this
}
};
This causes notes from sparse as well:
note: in included file:
include/uapi/sound/sof/header.h:56:38: warning: array of flexible structures
To help the compiler to understand this, we should convert it to u8 array.
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
The sof_debug value is set by the user, developer intentionally. To save time on figuring out what value has been passed to the kernel by the user, developer, print it out if it is not 0. Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Other topology ops have been treated as optional, including the route_free. Handle the route_setup in a conforming way as optional callback. Note: we do not have checks for the callbacks itself which makes them all optional in practice. Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
The core treats all function pointer in sof_ipc_tplg_ops as optional. Update the documentation to reflect this. Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
In preparation to a case when the DSP is not used. In this case the IPC communication itself has no meaning and we might not even have sdev->ipc allocated at all. The sof_ipc_get_ops() macro can be used to get a named IPC ops struct or return NULL if the sdev->ipc is not allocated. Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
The IPC ops are optional, but they require that the ops struct is to be allocated with all callbacks set to NULL. Update the code to extend the optionality to: sdev->ipc == NULL sdev->ipc->ops == NULL sdev->ipc->ops->[pcm] == NULL sdev->ipc->ops->[pcm]->ops == NULL (treated optional currently) Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
The IPC ops are optional, but they require that the ops struct is to be allocated with all callbacks set to NULL. Update the code to extend the optionality to: sdev->ipc == NULL sdev->ipc->ops == NULL sdev->ipc->ops->[tplg] == NULL sdev->ipc->ops->[tplg]->control == NULL sdev->ipc->ops->[tplg]->control->ops == NULL (treated optional currently) Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
The IPC ops are optional, but they require that the ops struct is to be allocated with all callbacks set to NULL. Update the code to extend the optionality to: sdev->ipc == NULL sdev->ipc->ops == NULL sdev->ipc->ops->[ops_group] == NULL sdev->ipc->ops->[pcmops_group]->ops == NULL (treated optional currently) Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
The IPC ops are optional, but they require that the ops struct is to be allocated with all callbacks set to NULL. Update the code to extend the optionality to: sdev->ipc == NULL sdev->ipc->ops == NULL sdev->ipc->ops->[tplg] == NULL sdev->ipc->ops->[tplg]->ops == NULL (treated optional currently) At the same time standardize the naming of the ops pointer to tplg_ops Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
The IPC ops are optional, but they require that the ops struct is to be allocated with all callbacks set to NULL. Update the code to extend the optionality to: sdev->ipc == NULL sdev->ipc->ops == NULL sdev->ipc->ops->[pm/tplg] == NULL (treated optional for pm currently) sdev->ipc->ops->[pm/tplg]->ops == NULL (treated optional currently) Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
The code treats the fw_tracing as optional feature but the documentation was not reflecting this. Correct it by explicitly stating that the fw_tracing is optional. Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
For the sake of safety use the sof_ipc_get_ops() to fetch the fw_tracing ops to avoid cases when either sdev->ipc or sdev->ipc->ops might be NULL. Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
If the sdev->fw_trace_is_supported is true then we must have the fw_tracing ops set, no need to check again. Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
To make sure that a correct target state is determined even if the DSP failed to boot or the firmware crashed or the suspend happens when the fw_state is not SOF_FW_BOOT_COMPLETE we must select the correct target state earlier. 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>
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>
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 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 (tested) on them. Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
set the dspless_mode_supported flag to true for tgl family to allow DSPless mode. 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>
Collaborator
Author
Right, I did have a clean branch, but by mistake I opened the PR on my WIP branch, I will close this and open a new one, addressing the comments so far. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 (tested) on them.