Skip to content

Conversation

@johnylin76
Copy link
Contributor

Fixes Speakers audio issue by aligning bitwidth format of DAI config to 24-bit as SSP config, which has been modified by commit c8fe192 <topology1: change audio format to 24 bit for rt1019>

This commit fixes Speakers audio issue by aligning bitwidth format
of DAI config to 24-bit as SSP config, which has been modified by
commit c8fe192 <topology1: change audio format to 24 bit for rt1019>

Signed-off-by: Pin-chih Lin <johnylin@google.com>
@johnylin76
Copy link
Contributor Author

Hi @Vamshigopal PTAL. Thanks.

@terry182
Copy link

Can confirm this PR fixes the audio distortion issue with ADL and ADL-N chromebooks using rt1019-rt5682.tplg.

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 @johnylin76 !

"sof-tgl-max98357a-rt5682\;sof-adl-rt1019-rt5682\;-DCODEC=RT1019\;-DFMT=s16le\;-DPLATFORM=adl\;-DLINUX_MACHINE_DRIVER=sof_rt5682\;-DAMP_SSP=1\;-DSPK_MIC_PERIOD_US=10000"
"sof-tgl-max98357a-rt5682\;sof-adl-rt1019-rt5682-waves\;-DCODEC=RT1019\;-DFMT=s16le\;-DPLATFORM=adl\;-DLINUX_MACHINE_DRIVER=sof_rt5682\;-DAMP_SSP=1\;-DWAVES"
"sof-tgl-max98357a-rt5682\;sof-adl-rt1019-rt5682\;-DCODEC=RT1019\;-DFMT=s24le\;-DPLATFORM=adl\;-DLINUX_MACHINE_DRIVER=sof_rt5682\;-DAMP_SSP=1\;-DSPK_MIC_PERIOD_US=10000"
"sof-tgl-max98357a-rt5682\;sof-adl-rt1019-rt5682-waves\;-DCODEC=RT1019\;-DFMT=s24le\;-DPLATFORM=adl\;-DLINUX_MACHINE_DRIVER=sof_rt5682\;-DAMP_SSP=1\;-DWAVES"
Copy link
Member

Choose a reason for hiding this comment

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

the question is: why do we have two places where the DAI format is configured?

This seems like a real problem but shouldn't we fix the macros instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree on this.
Although I'm not sure if there's exception, the configuration of SSP AMP and the corresponding DAI format will be determined by the codec. Currently they are defined separately (CODEC determines SSP_CONFIG, FMT specifies DAI). To avoid the future bugs, the implementation could be modified by bundling DAI format and SSP config pre-defined for each AMP codec driver.

However I would consider it to be done by another commit. We may need this quick fix to land in main and rpl-001 since it blocks the production build for rpl-001.

@plbossart WDYT?

Copy link
Member

Choose a reason for hiding this comment

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

ok

Copy link
Member

Choose a reason for hiding this comment

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

@johnylin76 please file an issue so that this problem is tracked and not lost.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks. I created #6961

@Vamshigopal
Copy link
Contributor

Hi @Vamshigopal PTAL. Thanks.

LGTM, Thanks @johnylin76 !

@lgirdwood
Copy link
Member

@johnylin76 I'm seeing lots of capture errors on ADL in CI. Pls see https://sof-ci.01.org/sofpr/PR6956/build3375/devicetest/index.html?model=ADLP_RVP_SDW_IPC4ZPH&testcase=check-capture-10sec
Are we missing a patch ?

@kv2019i
Copy link
Collaborator

kv2019i commented Jan 23, 2023

@lgirdwood https://sof-ci.01.org/sofpr/PR6956/build3375/devicetest/index.html seems like the earlier regression with SDW tplgs. Let me run the CI again, we should be clean with mainline now again.

@kv2019i
Copy link
Collaborator

kv2019i commented Jan 23, 2023

SOFCI TEST

@kv2019i
Copy link
Collaborator

kv2019i commented Jan 23, 2023

All good now, proceeding with merge.

@kv2019i kv2019i merged commit 091948c into thesofproject:main Jan 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants