Uh oh!
There was an error while loading. Please reload this page.
topology: tgl-max98373-rt5682: adjust channel usage for sdw DSM support - #2987
Conversation
RanderWang
commented
May 28, 2020
@ClarexZhou please include this PR for release, thanks! |
There was a problem hiding this comment.
that's still too much, we can only support 4 separate PCM transfers on a single link.
There was a problem hiding this comment.
@plbossart I tested it on the TGL production. And 4 separate PCM transfers : two channels playback & two channels capture ? As you know, the channel number for capture is double size of playback for IV feedback.
There was a problem hiding this comment.
@plbossart yes, it is impossible for 32bit * 6. 32bit PCM stream is used in DSM feature, but DAI config is set to 24bit (another story), so my PR works.
There was a problem hiding this comment.
This has nothing to do with bandwidth, word length or channel count. At the ALH level, we can only do 4 DMA transfers to/from PDI_BD PCM targets.
#defineDAI_NUM_ALH_BI_DIR_LINKS_GROUP 6
staticconstuint8_talh_handshake_map[64] = {
-1, /* 0 - INVALID */-1, /* 1 - INVALID */-1, /* 2 - INVALID */-1, /* 3 - INVALID */-1, /* 4 - INVALID */-1, /* 5 - INVALID */-1, /* 6 - INVALID */22, /* 7 - BIDIRECTIONAL */23, /* 8 - BIDIRECTIONAL */24, /* 9 - BIDIRECTIONAL */25, /* 10 - BIDIRECTIONAL */26, /* 11 - BIDIRECTIONAL */27, /* 12 - BIDIRECTIONAL */We can only use e.g. 9..12 for PCM transfers. I am not sure how you managed to transfer 6 independent streams since it's not aligned with the documentation. Can you describe which ALH stream numbers you used?
There was a problem hiding this comment.
@plbossart Iet's make it clear. There are only two PCMs in DSM feature: one is for playback (2ch) and another one is for capture (4ch).
There was a problem hiding this comment.
I was talking about ALH/DMA transfers of PCM data, your reply does not answer to my question.
There was a problem hiding this comment.
@plbossart I don't need to transfer 6 independent streams, right ? There are only two streams, so 32, /* 23 - BIDIRECTIONAL / is used for playback and 33, / 24 - BIDIRECTIONAL */ is used for capture.
There was a problem hiding this comment.
I dumped the stream id in driver, 32 /* 23 - BIDIRECTIONAL / is used for playback and 33 /* 24 - BIDIRECTIONAL / is used for capture ( link1).
There was a problem hiding this comment.
Got it now from the comments, best to say "FEEDBACK" instead of "FB" in the naming.
RanderWang
commented
May 29, 2020
update my commit message, thanks! |
keyonjie
commented
Jun 1, 2020
@RanderWang Is it possible to split them to 2 links if the customer still need 4ch playback and 8ch capture? |
RanderWang
commented
Jun 1, 2020
it is more complex. we need to add support in FW & kernel & topology. The key issue is shortage of such platform. @plbossart Do you have any conclusion about this feature ? Thanks! |
aiChaoSONG
commented
Jun 1, 2020
SOFCI TEST |
RanderWang
commented
Jun 1, 2020
failed cases were not caused by this PR for this topology is not used by CI |
lgirdwood
commented
Jun 11, 2020
@plbossart is this good for you now ? |
plbossart
left a comment
There was a problem hiding this comment.
I am not going to lay on the tracks for this but allow me to state that RX_CHANNELS and FB_CHANNELS are poorly defined. The amp provides I/V feedback, and at some point there's an echo reference generated, so I am not sure what RX and FB refer to.
lgirdwood
commented
Jun 12, 2020
Same here, @RanderWang can you explain what RX and FB are meaning here. We can probably make this easier to read with some name changes. |
RanderWang
commented
Jun 15, 2020
@lgirdwood . update comments in topology. Thanks! |
plbossart
left a comment
There was a problem hiding this comment.
Comments are not helping @RanderWang
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
@plbossart copy the comment in sof-smart-amplifier.m4
There was a problem hiding this comment.
FB_CHANNELS is used in mux input/output configuration, please check pipe-amp-ref-capture.m4. I spent almost an hour to check the FW but I didn't find anything about it. Then I consulted to a smart-amp expert. The FW patch is still not merged in master ! It is only adopted in release branch. According to tgl-006-drop-stable, the FB channel is the number of capture stream (I/V) sent to user mode application.
940f201 to
eaf4287Compare
plbossart
left a comment
There was a problem hiding this comment.
still not sure what 'feedback' is
Uh oh!
There was an error while loading. Please reload this page.
RanderWang
commented
Jun 24, 2020
update comments based on comments from @keyonjie. Thanks! |
For original design, there are 4 channels 24bit stream for playback and 8 channels 24bit IV feedback for capture stream. This requirement exhausts all resource on single link. Now adjust it to a 2 channels stream for playback and a 4 channels stream for capture Signed-off-by: Rander Wang <rander.wang@intel.com>
lgirdwood
commented
Jun 24, 2020
@zrombel looks like CI stalled ? |
zrombel
commented
Jun 24, 2020
Build was re run and all test are PASS. |
plbossart
left a comment
There was a problem hiding this comment.
we'll need to rename RX and FB in a follow-up patch, this is just confusing
For original design, there are 4 channels 24bit stream for playback
and 8 channels 24bit IV feedback for capture. This requirement exhausts
all resource on single link. Now adjust it to 2ch stream for playback
and 4ch stream for capture
Signed-off-by: Rander Wang rander.wang@intel.com