Skip to content
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

drivers: dai: add DAI driver for Intel UAOL #69906

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

drivers: dai: add DAI driver for Intel UAOL #69906

wants to merge 2 commits into from

Conversation

tlissows
Copy link
Contributor

@tlissows tlissows commented Mar 7, 2024 •
edited
Loading

This PR adds a DAI driver for USB Audio Offload Link (UAOL) on Intel ACE2.0 and ACE3.0 platforms.

@zephyrbot zephyrbot added area: DAI platform: Intel ADSP Intel Audio platforms area: Xtensa Xtensa Architecture area: Devicetree Binding PR modifies or adds a Device Tree binding labels Mar 7, 2024
@tlissows tlissows force-pushed the uaol-dai-upstream branch from 2ad4f2e to 334330f Compare March 11, 2024 12:26
mwasko
mwasko previously approved these changes Mar 12, 2024
View reviewed changes
@kv2019i kv2019i requested review from dbaluta and removed request for softwarecki March 12, 2024 11:22
kv2019i
kv2019i previously approved these changes Mar 12, 2024
View reviewed changes
Copy link
Collaborator

@kv2019i kv2019i left a comment

Choose a reason for hiding this comment

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

That's a big PR to go through. Some minor comments inline, but nothing to block merge. The first commit affects other DAI users, so I added Daniel to review list.

include/zephyr/drivers/dai.h Outdated Show resolved Hide resolved
drivers/dai/intel/uaol/uaol.c Outdated Show resolved Hide resolved
drivers/dai/intel/uaol/uaol.c Outdated Show resolved Hide resolved
drivers/dai/intel/uaol/uaol.c Outdated Show resolved Hide resolved
drivers/dai/intel/uaol/uaol.c Outdated Show resolved Hide resolved
drivers/dai/intel/uaol/uaol_regs_ace1x.h Outdated Show resolved Hide resolved
@tlissows tlissows dismissed stale reviews from kv2019i and mwasko via f5d6684 March 13, 2024 12:43
@tlissows tlissows force-pushed the uaol-dai-upstream branch from 334330f to f5d6684 Compare March 13, 2024 12:43
mwasko
mwasko previously approved these changes Mar 13, 2024
View reviewed changes
kv2019i
kv2019i reviewed Mar 13, 2024
View reviewed changes
Copy link
Collaborator

@kv2019i kv2019i left a comment

Choose a reason for hiding this comment

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

Thanks @tlissows , looks good now! One question/comment I missed yesterday, can you check (see inline)?

include/zephyr/drivers/dai.h Outdated Show resolved Hide resolved
@marc-hb marc-hb removed request for tmleman and marc-hb June 14, 2024 21:44
andyross
andyross reviewed Jun 15, 2024
View reviewed changes
Copy link
Contributor

@andyross andyross left a comment

Choose a reason for hiding this comment

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

Some notes, though I didn't get all the way through. Mostly about API and interop issues, obviously I'm not qualified to talk about the hardware.

drivers/uaol/uaol_intel_adsp.c Outdated Show resolved Hide resolved
drivers/uaol/uaol_intel_adsp.c Outdated Show resolved Hide resolved
drivers/uaol/uaol_intel_adsp.c Outdated Show resolved Hide resolved
drivers/uaol/uaol_intel_adsp.c Outdated Show resolved Hide resolved
drivers/uaol/uaol_intel_adsp.c Outdated Show resolved Hide resolved
drivers/uaol/uaol_intel_adsp.c Outdated Show resolved Hide resolved
drivers/uaol/uaol_intel_adsp.c Outdated Show resolved Hide resolved
drivers/uaol/uaol_intel_adsp.c Outdated Show resolved Hide resolved
nashif
nashif previously approved these changes Jun 15, 2024
View reviewed changes
@nashif nashif dismissed their stale review June 15, 2024 01:10

approved prematurely.

pjdobrowolski
pjdobrowolski previously approved these changes Jun 17, 2024
View reviewed changes
@aescolar aescolar modified the milestones: v3.7.0, v4.0.0 Jun 17, 2024
@aescolar
Copy link
Member

Bulk changing the milestone from everything which did not make it to the v3.7.0 freeze deadline
and which is not labeled as a bug or documentation (or does not appear clearly as only containing
a bug fix or a documentation change).
If you disagree about this change please add the milestone again.

Please note that until rc2 only PRs containing bug fixes, docs and tests can be merged.
After that and until release only bug fixes and docs changes.

Exceptions require approval by the release team and a vote by the TSC.
https://docs.zephyrproject.org/latest/project/release_process.html#stabilization-phase

@tlissows tlissows dismissed stale reviews from pjdobrowolski and dnikodem via fbefe20 June 18, 2024 10:17
@tlissows tlissows force-pushed the uaol-dai-upstream branch from f4a6409 to fbefe20 Compare June 18, 2024 10:17
@tlissows tlissows requested a review from andyross June 18, 2024 14:25
tmleman
tmleman requested changes Jun 28, 2024
View reviewed changes
Copy link
Collaborator

@tmleman tmleman left a comment

Choose a reason for hiding this comment

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

Please prepare a pull request in SOF that points to this PR. Rebuilding this with SOF will avoid the need to return here with minor fixes after the merge. Currently, I am not sure, but previously in some SOF workflows, warnings were treated as errors.

drivers/dai/intel/uaol/uaol.c Show resolved Hide resolved
drivers/dai/intel/uaol/uaol.c Show resolved Hide resolved
drivers/dai/intel/uaol/uaol.c Show resolved Hide resolved
drivers/dai/intel/uaol/uaol-params-intel-ipc4.h Outdated Show resolved Hide resolved
tmleman
tmleman previously requested changes Jun 28, 2024
View reviewed changes
Copy link
Collaborator

@tmleman tmleman left a comment

Choose a reason for hiding this comment

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

The solution of moving the driver for UAOL to a lower level than DAI is okay. However, you have device power management implemented twice because of this. Is there a reason for this? In other DAIs, we have device power management because there is no driver below for the given interface. I think power management in drivers/uaol/uaol_intel_adsp.c would be sufficient. Additionally, the appropriate power domains should be added to the DTS (UAOL is in the hst_domain domain).

drivers/dai/intel/uaol/uaol.c Outdated Show resolved Hide resolved
@tlissows tlissows force-pushed the uaol-dai-upstream branch from fbefe20 to 2355eeb Compare July 18, 2024 08:12
@tmleman tmleman dismissed their stale review August 9, 2024 20:45

I am removing my change request because I won't be available in the near future and I don't want to unnecessarily block these changes.

@tlissows tlissows mentioned this pull request Oct 2, 2024
Open
@tlissows
Copy link
Contributor Author

tlissows commented Oct 2, 2024

The solution of moving the driver for UAOL to a lower level than DAI is okay. However, you have device power management implemented twice because of this. Is there a reason for this? In other DAIs, we have device power management because there is no driver below for the given interface. I think power management in drivers/uaol/uaol_intel_adsp.c would be sufficient. Additionally, the appropriate power domains should be added to the DTS (UAOL is in the hst_domain domain).

UAOL power domains added in DTS. As for power management, exposing it by UAOL DAI driver (in addition to UAOL IP driver) seems reasonable due to its ease of use on application side. As an example please see basefw_vendor_dma_control() function (in SOF), where you can find power mnanagement API called directly for DAI devices. Calls for UAOL IP devices would also be possible, but much more complicated. Some extra steps would be needed (like UAOL DAI -> IP mapping) thus creating an exceptional path for UAOL.

@tlissows
Copy link
Contributor Author

tlissows commented Oct 2, 2024

Removed support for ACE1.x platform from this PR. Also aligned to the changes introduced by commit include: dai: Introduce runtime DAI configuration update API.

tmleman
tmleman previously approved these changes Nov 4, 2024
View reviewed changes
Copy link
Collaborator

@tmleman tmleman left a comment

Choose a reason for hiding this comment

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

My earlier comments have been addressed.

@mmahadevan108 mmahadevan108 modified the milestones: v4.0.0, v4.1.0 Nov 8, 2024
@tlissows
drivers: uaol: add a driver for Intel UAOL IP
b7caec4 
This adds a driver for USB Audio Offload Link (UAOL) IP
on Intel ACE2.0 and ACE3.0 platforms.

Signed-off-by: Tomasz Lissowski <tomasz.lissowski@intel.com>
@tlissows
drivers: dai: intel: uaol: add a driver for Intel UAOL DAI
12cd681 
This adds a DAI driver for USB Audio Offload Link (UAOL)
individual streams on Intel ACE2.0 and ACE3.0 platforms.

Signed-off-by: Tomasz Lissowski <tomasz.lissowski@intel.com>
@tlissows
Copy link
Contributor Author

tlissows commented Dec 17, 2024 •
edited
Loading

Rebased on top of main.
Used WAIT_FOR() macro where applicable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@marc-hb marc-hb marc-hb left review comments

@tmleman tmleman tmleman left review comments

@pjdobrowolski pjdobrowolski pjdobrowolski left review comments

@dnikodem dnikodem dnikodem left review comments

@nashif nashif nashif left review comments

@dcpleung dcpleung Awaiting requested review from dcpleung

@marcinszkudlinski marcinszkudlinski Awaiting requested review from marcinszkudlinski

@abonislawski abonislawski Awaiting requested review from abonislawski

@ceolin ceolin Awaiting requested review from ceolin

@galak galak Awaiting requested review from galak

@juimonen juimonen Awaiting requested review from juimonen

@lgirdwood lgirdwood Awaiting requested review from lgirdwood

@lyakh lyakh Awaiting requested review from lyakh

@dbaluta dbaluta Awaiting requested review from dbaluta

@ujfalusi ujfalusi Awaiting requested review from ujfalusi

@kv2019i kv2019i Awaiting requested review from kv2019i

@mwasko mwasko Awaiting requested review from mwasko

@dabekjakub dabekjakub Awaiting requested review from dabekjakub

@andyross andyross Awaiting requested review from andyross

At least 2 approving reviews are required to merge this pull request.

Assignees

@kv2019i kv2019i

Labels
area: DAI area: Devicetree Binding PR modifies or adds a Device Tree binding area: Xtensa Xtensa Architecture platform: Intel ADSP Intel Audio platforms
Projects
None yet
Milestone
v4.1.0
Development

Successfully merging this pull request may close these issues.