-
Notifications
You must be signed in to change notification settings - Fork 325
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
[BUG] SoundWire topology: need to use 24 bits #8960
Comments
@plbossart @bardliao @ranj063 @ujfalusi what's the status for this today ? |
status is "not started, won't happen in Q2" |
As per Pierre's feedback, moving to v2.11. |
plbossart
added a commit
to plbossart/sof
that referenced
this issue
Jul 4, 2024
Using S32_LE wastes bandwdith for no good reason, we should use 24 bits on the link to maximize bus efficiency with the 9.6 MHz bus clock. FIXME: it's likely we need a kernel-side change as well to configure the hw_params to S24_LE, currently the stream/port is configured with sconfig.bps = snd_pcm_format_width(params_format(params)); Link: thesofproject#8960 Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
plbossart
added a commit
to plbossart/sof
that referenced
this issue
Jul 4, 2024
Using S32_LE wastes bandwdith for no good reason, we should use 24 bits on the link to maximize bus efficiency with the 9.6 MHz bus clock. There is no need for a kernel-side change, the dailink fixup already changes the dailink format based on the topology information. Link: thesofproject#8960 Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
plbossart
added a commit
to plbossart/sof
that referenced
this issue
Jul 5, 2024
Using S32_LE wastes bandwdith for no good reason, we should use 24 bits on the link to maximize bus efficiency with the 9.6 MHz bus clock. There is no need for a kernel-side change, the dailink fixup already changes the dailink format based on the topology information. Link: thesofproject#8960 Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
kv2019i
pushed a commit
that referenced
this issue
Jul 15, 2024
Using S32_LE wastes bandwdith for no good reason, we should use 24 bits on the link to maximize bus efficiency with the 9.6 MHz bus clock. There is no need for a kernel-side change, the dailink fixup already changes the dailink format based on the topology information. Link: #8960 Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
eddy1021
pushed a commit
to eddy1021/sof
that referenced
this issue
Jul 15, 2024
Using S32_LE wastes bandwdith for no good reason, we should use 24 bits on the link to maximize bus efficiency with the 9.6 MHz bus clock. There is no need for a kernel-side change, the dailink fixup already changes the dailink format based on the topology information. Link: thesofproject#8960 Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
@bardliao @plbossart Can you add a status update on this and propose new milestone (raise priority and get in 2.11 or push to 2.12)? |
@kv2019i @plbossart I think we can close this issue as #9282 has been merged. |
yeah, this is done already, closing |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Reported initially by @bardliao
All SoundWire topologies currently use S32_LE, which means all 32-bits of data will be pushed on the wire.
The problem is that the default frame shape for SoundWire is 50x4. The column0 is reserved for commands, so in theory we can have 3 streams of 24-bits each on column 1, 2, 3 respectively.
But since the topologies use S32_LE, we can only support 2 streams: conventional arithmetic tells us there's no way to fit 64-bits of data in 50 bits.
That's a problem for integrated devices, e.g. if you want to have amp+headset at the same time on the same link. We need to switch to 24 valid bits to avoid artificial bandwidth limitations.
We certainly need to avoid the use of 32 bits for topology2 solutions. It needs to be determined if topology1 need this change or not.
The text was updated successfully, but these errors were encountered: