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.

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

deepbuffer PCM not following period size setting #4795

Open
kv2019i opened this issue Jan 22, 2024 · 7 comments
Open

deepbuffer PCM not following period size setting #4795

kv2019i opened this issue Jan 22, 2024 · 7 comments
Labels
bug Something isn't working MTL Applies to Meteor Lake platform.

Comments

@kv2019i
Copy link
Collaborator

kv2019i commented Jan 22, 2024

Testing with a new extension to alsa-conformance-test for delay reporting (https://github.com/kv2019i/cros-audiotest/tree/topic/pcm-delay-debug), shows unexpected wake-up behavior for the SOF deepbuffer PCM on Intel MTL platform.

Environment

kernel: 66ee247 (sof-dev)
SOF: 6c188298b958 (SOF main) - thesofproject/sof#8756 needed for correct delay values but not mandatory to reproduce this bug
platform: MTL
topoplogy: sof-mtl-rt713-l0-rt1316-l12.tplg

Expected outcome

The driver should wake up user-space as per the configured period-size.

Additionally, INFO_BATCH should be set to indicate hw_ptr will move in jumps (see thesofproject/sof#8717 ).

Actual outcome

If application sets a 10ms period size and 100ms buffer-size, the hw-ptr will jump 90ms at a time.

Example log (with above alsa-conformance-test version). Note, below log with FW patch thesofproject/sof#8756 to address #4781 -- otherwise the delay values are not correct. Not mandatory to reproduce this bug.

$ aplay -l |grep -A2 "device 31"
card 0: sofsoundwire [sof-soundwire], device 31: Deepbuffer Jack Out (*) []
  Subdevices: 1/1
  Subdevice #0: subdevice #0
$ alsa_conformance_test -Phw:0,31 -p 480 -B 2400 --merge_threshold_sz=4800 -d 60 --debug 
TIME_DIFF(s)    HW_LEVEL     PLAYED       DIFF               RATE      DELAY PLAYED_ADJ       DIFF
0.000060024            0       4800       4800    79968012.794882 7493989779944510264 -7493989779944505464 -7493989779944505464
0.095043928         4552       5048        248        2609.319766       4913       4687 7493989779944510151
0.000023466         3346       6254       1206    51393505.497315       4927       4673        -14 [Merged]
0.000021303         2296       7304       1050    49288832.558795       4921       4679          6 [Merged]
0.000019454         3744       8256        952    48935951.475275       7321       4679          0 [Merged]
0.000018708         2840       9160        904    48321573.658328       7323       4677         -2 [Merged]
0.000025679         2592       9408        248     9657696.950816       7513       4487       -190 [Merged]
0.095874426         2544       9456         48         500.654888       2681       9319       4832
0.000022952         1374      10626       1170    50975949.808296       2715       9285        -34 [Merged]
0.000020586         2760      11640       1014    49256776.450015       5121       9279         -6 [Merged]
0.000020071         1776      12624        984    49025957.849634       5113       9287          8 [Merged]
0.000020940         3144      13656       1032    49283667.621777       7513       9287          0 [Merged]
0.000026964         2784      14016        360    13351134.846462       7705       9095       -192 [Merged]
0.095880649         2544      14256        240        2503.111968       2873      13927       4832

Notes

With defaults settings alsa-conformance-test will hit undderuns with the deepbuffer PCM and the results will not be valid. To avoid this, the block size (-B 2400) is manually set. This is the amount of data user-space writes at a time to PCM and is separate from the configured period-size.

@kv2019i kv2019i added bug Something isn't working MTL Applies to Meteor Lake platform. labels Jan 22, 2024
@kv2019i
Copy link
Collaborator Author

kv2019i commented Jan 22, 2024

FYI @RanderWang @ranj063 @ujfalusi @plbossart -- the deepbuffer PCM has some unexpected behaviour towards the user-space. Not necessarily a problem for apps that can handle this, but e.g. this won't pass the conformance test.

@plbossart
Copy link
Member

@kv2019i for my education, why would an application set a 100ms buffer and then ask to be awaken 10 times? The canonical ALSA behavior is for 4 periods, and the goal would actually be to use no period wake-ups and instead wake-up with a timer at the 80 or 90ms watermark.

Just trying to understand if we are dealing with a validation corner case that has no direct bearing on actual apps, or something with a direct app/product impact.

@yongzhi1
Copy link

yongzhi1 commented Feb 28, 2024

FYI @RanderWang @ranj063 @ujfalusi @plbossart -- the deepbuffer PCM has some unexpected behaviour towards the user-space. Not necessarily a problem for apps that can handle this, but e.g. this won't pass the conformance test.

The audio position has an impact for AV synchronization, assume the latency is constant if queried every 100ms, the conformance rate test should pass when --merge_threshold_t set to 0.1 sec right? but it still fails.

@plbossart
Copy link
Member

@kv2019i @ujfalusi @yongzhi1 I think this is solved, no? can we close?

@ujfalusi
Copy link
Collaborator

@plbossart, with an improved alsaconformance test the reported delay is taken into account for the rate validation and it passes fine:
https://github.com/ujfalusi/chromeos-audiotest/tree/topic/pcm-delay

I really don't know how that can be upstreamed back to Chrome...

@plbossart
Copy link
Member

@yongzhi1 @sathya-nujella can you help propagate the patches back to the Chrome repo?

@yongzhi1
Copy link

yongzhi1 commented May 2, 2024

@yongzhi1 @sathya-nujella can you help propagate the patches back to the Chrome repo?

Sure, maybe after the kernel series get merged to Google tree.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working MTL Applies to Meteor Lake platform.
Projects
None yet
Development

No branches or pull requests

4 participants