Skip to content

topology2: intel: add generic Bluetooth offload pipelines #6971

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
kv2019i merged 1 commit into from
Jan 27, 2023
Merged

topology2: intel: add generic Bluetooth offload pipelines #6971

kv2019i merged 1 commit into from
Jan 27, 2023

Conversation

@kv2019i
Copy link
Collaborator

@kv2019i kv2019i commented Jan 19, 2023

Add generic Bluetooth pipeline definitions that matches what we've had with topology1. Augment passthrough pipelines to allow use of Bluetooth.

ranj063
ranj063 reviewed Jan 19, 2023
View reviewed changes
@kv2019i
Copy link
Collaborator Author

kv2019i commented Jan 23, 2023

V2 (Jan 23rd) update:

  • removed changes to generic passthrough pipeline definitions, override the values in bt-generic.conf instead
  • fix A2DP profile support (48kHz/stereo) -> in V1, the FE->BE connection converted samples incorrectly S32_LE .. the SSP driver stripped the LSB bits so no runtime error was raised, but this is still wrong

@kv2019i
Copy link
Collaborator Author

kv2019i commented Jan 23, 2023

@ranj063 I probably should fix this before merge to align with #6627 ... right?

@ranj063
Copy link
Collaborator

ranj063 commented Jan 23, 2023

@ranj063 I probably should fix this before merge to align with #6627 ... right?

@kv2019i yes thats right. We should

@marc-hb
Copy link
Collaborator

marc-hb commented Jan 23, 2023 •
edited
Loading

Following the long and winded road starting from https://sof-ci.01.org/sofpr/PR6971/build3487/devicetest/index.html (internal ID 20119), I found that the reason why ADLP_RVP_SDW_IPC4ZPH failed is as usual NOT because that model was "Not Available" but because:

09:02:56  [2023-01-23 17:02:55 UTC][SOF-SH] Now time: 2023-01-23T17:02:55+00:00 | Device time: 2023-01-23T17:02:55+00:00
09:02:57  [2023-01-23 17:02:56 UTC][SOF-SH] ERROR: Failed to retrieve Topology resource from
       [http://sof-storage.ostc.intel.com/pr/sof/PR6971-3487/build3487/tplg/topology2/avs-tplg/sof-adl-rt711-4ch.tplg](http://sof-storage.ostc.intel.com/pr/sof/PR6971-3487/build3487/tplg/topology2/avs-tplg/sof-adl-rt711-4ch.tplg

(/job/testflow/62327)

The directory http://sof-storage.ostc.intel.com/pr/sof/PR6971-3487/build3487/tplg/topology2/avs-tplg/ does exist.

Could this be caused by this PR?

@kv2019i
topology2: add generic Bluetooth offload pipeline
d3902b5 
Add generic Bluetooth offload pipeline and two example
topologies based on cavs-nocodec that are using the new
Bluetooth pipelines.

1) cavs-nocodec-bt.conf

Example topology that matches configuration used on may
Intel SOF boards (SSP2, Bluetooth SCO and A2DP configurations).

2) cavs-nocodec-bt-lbm.conf

Variant of first topology that configures all mode with
SSP loopback mode, allowing to verify the pipeline operation
by capturing the audio back from the same PCM.

Requires alsa-utils patch "topology: nhlt: intel: ssp: fix obvious typo
for 'codec_provider'" to correctly generate the NHLT blobs for 8/16khz
modes.

Link: alsa-project/alsa-utils#174
Co-developed-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Signed-off-by: Jaska Uimonen <jaska.uimonen@intel.com>
@kv2019i
Copy link
Collaborator Author

kv2019i commented Jan 24, 2023

V3 (Jan 24th) update:

juimonen
juimonen approved these changes Jan 24, 2023
View reviewed changes
Copy link

@juimonen juimonen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pierre's typo patch went upstream, so not sure if needed to mention that in commit message. LGTM.

@plbossart
Copy link
Member

plbossart commented Jan 24, 2023

Pierre's typo patch went upstream, so not sure if needed to mention that in commit message. LGTM.

it does impact the alsa-plugin tools one would think, so the docker image should be updated, no? @kv2019i @juimonen @fredoh9

@juimonen
Copy link

juimonen commented Jan 24, 2023

Pierre's typo patch went upstream, so not sure if needed to mention that in commit message. LGTM.

it does impact the alsa-plugin tools one would think, so the docker image should be updated, no? @kv2019i @juimonen @fredoh9

yes definitely, need update to latest upstream. Just meant that Kai's original message was there perhaps to inform that the needed patch is not yet in upstream alsa-utils head. But now it is -> so we need update also.

@kv2019i
Copy link
Collaborator Author

kv2019i commented Jan 27, 2023

Let's proceed with merge.

@kv2019i kv2019i merged commit 4476171 into thesofproject:main Jan 27, 2023
@marc-hb marc-hb mentioned this pull request Jan 27, 2023
Merged
@marc-hb
Copy link
Collaborator

marc-hb commented Jan 27, 2023 •
edited
Loading

There was "logical" conflict between this 4476171 and ae8ca22. They both passed separately but not together - and CI never tested them together.

This sort of "logical conflict" is very rare and too hard to catch in CI, not worth the effort. The mitigations are:

  • Do an empty git commit --amend -C HEAD and force-push when the PR hasn't been tested for a while. Don't rebase as it makes it extremely difficult to see what changed. Most CIs merge before testing.
  • Keep an eye on the git log and communicate more! :-)

@marc-hb marc-hb mentioned this pull request Jan 27, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ranj063 ranj063 ranj063 approved these changes

@lgirdwood lgirdwood lgirdwood approved these changes

@udaymb udaymb Awaiting requested review from udaymb

@marc-hb marc-hb Awaiting requested review from marc-hb

@aborisovich aborisovich Awaiting requested review from aborisovich

@plbossart plbossart Awaiting requested review from plbossart

@mmaka1 mmaka1 Awaiting requested review from mmaka1

@lbetlej lbetlej Awaiting requested review from lbetlej

@dbaluta dbaluta Awaiting requested review from dbaluta

+1 more reviewer

@juimonen juimonen juimonen approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@kv2019i @ranj063 @marc-hb @plbossart @juimonen @lgirdwood