Skip to content

[Temporarily Inactive] zephyr: alh: add ALH zephyr driver - #5727

Closed
abonislawski wants to merge 1 commit into
thesofproject:mainfrom
abonislawski:zephyr_alh_upstream
Closed

[Temporarily Inactive] zephyr: alh: add ALH zephyr driver#5727
abonislawski wants to merge 1 commit into
thesofproject:mainfrom
abonislawski:zephyr_alh_upstream

Conversation

@abonislawski

Copy link
Copy Markdown
Member

Rename dai funcs, add zephyr alh driver,
add get_stream_id to dai driver
Switch to ALH zephyr driver in zephyr build

based on SSP example https://github.com/juimonen/sof/commits/ssp-wip

Rename dai funcs, add zephyr alh driver,
add get_stream_id to dai driver
Switch to ALH zephyr driver in zephyr build
Signed-off-by: Adrian Bonislawski <adrian.bonislawski@intel.com>
Comment threadsrc/ipc/ipc4/dai.c
* not during topology parsing.
*/
dd->stream_id = alh->params.stream_id;
dd->stream_id = dai_get_stream_id(dai_p, 0);//alh->params.stream_id;

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Temporarily added dai_get_stream_id, not sure if this is the correct approach (feel free to comment) but we need to address this as alh private data was used here

break;
default:
dai_warn(dai,"%d not supported format", cfg->word_size);
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

indentation - like in SSP

@lgirdwoodlgirdwood left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@juimonen@abonislawski we need to do the abstraction at the layer above for both DMA and DAIs. i.e.for DAIs like ALH and SSP we need to

  1. rename src/audio/dai.c to src/audio/dai-xtos.c
  2. create a new file called src/audio/dai.c that maps the SOF dai module to Zephyr DAI driver APIs
    Likewise for the DMAs
    This avoids all the alh-zephyr.c and ssp-zephyr.c (+more) wrappers - as we map/wrap at the layer above.

@juimonen

Copy link
Copy Markdown

@juimonen@abonislawski we need to do the abstraction at the layer above for both DMA and DAIs. i.e.for DAIs like ALH and SSP we need to

  1. rename src/audio/dai.c to src/audio/dai-xtos.c
  2. create a new file called src/audio/dai.c that maps the SOF dai module to Zephyr DAI driver APIs
    Likewise for the DMAs
    This avoids all the alh-zephyr.c and ssp-zephyr.c (+more) wrappers - as we map/wrap at the layer above.

@lgirdwood I see we could unify ssp-zephyr and alh-zephyr etc. into a single wrapper file, but that's about it....??? the dai_probe etc name clash with zephyr needs to be resolved anyway.

@kv2019ikv2019i left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This modifies cAVS2.5 build which is probably not intentional (and can't be merged as the matching drivers and board changes are not submitted to Zephyr upstream for cAVS2.5).

I'm guessing this is better done via #5731 , right ?

Comment threadzephyr/CMakeLists.txt

zephyr_library_sources_ifdef(CONFIG_INTEL_ALH
${SOF_DRIVERS_PATH}/intel/alh.c
${SOF_DRIVERS_PATH}/intel/alh/alh-zephyr.c

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think this is a problem. If we change cAVS2.5 to use the Zephyr ALH driver, then we have to submit matching changes to Zephyr upstream (for cAVS2.5).

@lgirdwoodlgirdwood added the P1 Blocker bugs or important features label May 24, 2022

@lgirdwoodlgirdwood left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Needs to be abstracted as follows (and this looks like it's half done here).

  1. rename alh.c to alh-legacy.c (and build will use this for xtos).
  2. Can we have a src/lib/dai-common.c, dail-legacy.c and dai-zephyr.c

@abonislawski

Copy link
Copy Markdown
MemberAuthor

Just to make sure everyone is up to date: this PR is using old way with zephyr wrappers in sof, it is not needed anymore as we are switching to native zephyr api in DAI PR where alh part from this PR is already included
or if the DAI PR will be merged without alh then this PR will just extend DAI PR with alh

@lyakhlyakh changed the title zephyr: alh: add ALH zephyr driver[Temporarily Inactive] zephyr: alh: add ALH zephyr driverJun 1, 2022
@lgirdwoodlgirdwood modified the milestone: v2.3Jun 1, 2022
@lgirdwood

Copy link
Copy Markdown
Member

Just to make sure everyone is up to date: this PR is using old way with zephyr wrappers in sof, it is not needed anymore as we are switching to native zephyr api in DAI PR where alh part from this PR is already included or if the DAI PR will be merged without alh then this PR will just extend DAI PR with alh

We will be using native Zephyr APIs in the same manner as the DMA API that has been merged so does this need closed ?

@abonislawski

Copy link
Copy Markdown
MemberAuthor

Yes, I will reopen if needed

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

Labels

P1Blocker bugs or important features

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@abonislawski@juimonen@lgirdwood@lyakh@kv2019i