Skip to content

dai: overwrite hardware frame_fmt parameter with private frame_fmt #2421

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
lgirdwood merged 1 commit into from
Mar 2, 2020
Merged

dai: overwrite hardware frame_fmt parameter with private frame_fmt #2421

lgirdwood merged 1 commit into from
Mar 2, 2020

Conversation

@bkokoszx
Copy link
Collaborator

@bkokoszx bkokoszx commented Feb 21, 2020 •
edited
Loading

Overwrite frame_fmt hardware parameters with DAI private frame_fmt
in dai_comp_get_hw_params() function as DAI component is able to
convert stream with different frame_fmt's.

e.g.
In following topology:
DAI (DMIC, 32 bit) --> IIR ---> host (16 bit)

We have two components that potentially can make frame_fmt conversion -
DAI and EQ_IIR. With above patch we can choose which component will
make this conversion. If we create DAI component with frame_fmt equal
to SOF_IPC_FRAME_S32_LE in struct sof_ipc_comp_config, conversion will
happen on IIR component, otherwise if we create DAI component with
SOF_IPC_FRAME_S16_LE format, conversion will happen on DAI.

Signed-off-by: Bartosz Kokoszko bartoszx.kokoszko@linux.intel.com

@bkokoszx bkokoszx mentioned this pull request Feb 21, 2020
Merged
lgirdwood
lgirdwood reviewed Feb 21, 2020
View reviewed changes
Copy link
Member

@lgirdwood lgirdwood left a comment

Choose a reason for hiding this comment

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

Can you give me an example use case here.

@bkokoszx
Copy link
Collaborator Author

bkokoszx commented Feb 21, 2020

@lgirdwood
I've updated PR description.

plbossart
plbossart reviewed Feb 21, 2020
View reviewed changes
src/audio/dai.c Outdated
Copy link
Member

@plbossart plbossart Feb 21, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

@bkokoszx can you clarify for me the commit message and comment above "DAI component is able to convert stream with different frame_fmt's"?
It was my understanding that the DAI operates with a fixed format, and conversions are e.g. handled in a volume component. What am I missing?

Copy link
Collaborator Author

@bkokoszx bkokoszx Feb 24, 2020

Choose a reason for hiding this comment

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

@plbossart
For some time the DAI component has been able to make frame_fmt conversion. We do not need volume component to do that. This capability has been added in PR #2029.

Copy link
Member

@lgirdwood lgirdwood Feb 24, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

@bkokoszx best to add more context to this comment. This is about back propagating the DAI format back through the pipeline so that any converter component can convert from format X -> Y if needed.
I would also make this a static inline since it will be used as part of runtime by multi-DAI support.

@lgirdwood
Copy link
Member

lgirdwood commented Feb 24, 2020

@bkokoszx how do we choose which componet will do the format conversion ? e.g from your example

In following topology:
DAI (DMIC, 32 bit) --> IIR ---> host (16 bit)

We have two components that potentially can make frame_fmt conversion -
DAI and EQ_IIR```

@bkokoszx
Copy link
Collaborator Author

bkokoszx commented Feb 24, 2020

@lgirdwood
DAI component has bigger priority. I've updated PR description, which I think, answers your question. :)

lgirdwood
lgirdwood requested changes Feb 24, 2020
View reviewed changes
src/audio/dai.c Outdated
Copy link
Member

@lgirdwood lgirdwood Feb 24, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

@bkokoszx best to add more context to this comment. This is about back propagating the DAI format back through the pipeline so that any converter component can convert from format X -> Y if needed.
I would also make this a static inline since it will be used as part of runtime by multi-DAI support.

@bkokoszx bkokoszx force-pushed the dai-ovewrite-hw-frame-fmt branch from 44b30fe to 3b5b926 Compare February 25, 2020 11:58
@bkokoszx
Copy link
Collaborator Author

bkokoszx commented Feb 25, 2020

@lgirdwood
I have just updated PR.

@lgirdwood
Copy link
Member

lgirdwood commented Feb 26, 2020

@bkokoszx can you fix conflicts.

@bkokoszx bkokoszx force-pushed the dai-ovewrite-hw-frame-fmt branch from 3b5b926 to 2e2a64c Compare February 27, 2020 07:55
@bkokoszx
Copy link
Collaborator Author

bkokoszx commented Feb 27, 2020

@lgirdwood
Updated

@lgirdwood
Copy link
Member

lgirdwood commented Feb 27, 2020

SOFCI TEST

@lgirdwood
Copy link
Member

lgirdwood commented Feb 28, 2020

@bkokoszx I'm seeing the known KD CI failure, but I'm also seeing ALH loopback failures. Can you check.

@bkokoszx
Copy link
Collaborator Author

bkokoszx commented Mar 1, 2020

SOFCI TEST

@lgirdwood
Copy link
Member

lgirdwood commented Mar 1, 2020

@bkokoszx seeing a new fail on internal CI with WHL. Can you check.

@bkokoszx
Copy link
Collaborator Author

bkokoszx commented Mar 2, 2020 •
edited
Loading

@lgirdwood
TestLoadFwExtended and TestLoadFwBasic tests failed also for other PR's. I guess, there was something wrong with WHL CI platform. Platform was rebooted - this PR has been retriggered.

@bkokoszx
Copy link
Collaborator Author

bkokoszx commented Mar 2, 2020

I will check TestAlhLoopbackVolume48000Hz16b16b8ch test

@lgirdwood
Copy link
Member

lgirdwood commented Mar 2, 2020

@bkokoszx ok great, btw we are still seeing the KD test regression for all PRs now. @wwittbrx has this been bisected yet ? or are we expecting a fix from @mrajwa or @akloniex ?

@bkokoszx
Copy link
Collaborator Author

bkokoszx commented Mar 2, 2020

@lgirdwood
KD regression should be fixed by @tlauda PR #2454

juimonen
juimonen approved these changes Mar 2, 2020
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.

LGTM, I can only comment code changes, not the underlying functionality. Anyway conversation seems to be settled.

@bkokoszx
dai: overwrite hardware frame_fmt parameter with private frame_fmt
e8733fb 
Overwrite frame_fmt hardware parameters with DAI private frame_fmt
in dai_comp_get_hw_params() function as DAI component is able to
convert stream with different frame_fmt's

Signed-off-by: Bartosz Kokoszko <bartoszx.kokoszko@linux.intel.com>
@bkokoszx bkokoszx force-pushed the dai-ovewrite-hw-frame-fmt branch from 2e2a64c to e8733fb Compare March 2, 2020 12:02
@bkokoszx
Copy link
Collaborator Author

bkokoszx commented Mar 2, 2020

@lgirdwood
I've updated PR. CI passes.

@lgirdwood lgirdwood merged commit 01b4128 into thesofproject:master Mar 2, 2020
@bkokoszx bkokoszx deleted the dai-ovewrite-hw-frame-fmt branch March 10, 2020 12:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@plbossart plbossart plbossart left review comments

@lgirdwood lgirdwood lgirdwood approved these changes

@dbaluta dbaluta Awaiting requested review from dbaluta

@jajanusz jajanusz Awaiting requested review from jajanusz

@lbetlej lbetlej Awaiting requested review from lbetlej

@mmaka1 mmaka1 Awaiting requested review from mmaka1

@tlauda tlauda Awaiting requested review from tlauda

+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.

4 participants

@bkokoszx @lgirdwood @juimonen @plbossart