Repository navigation
topology: Update CML topologies - #1774
Conversation
plbossart
left a comment
plbossart
left a comment
There was a problem hiding this comment.
See comments below, I believe the support for the max98357a was a botched job and we need to think more when we have firmware/topologies dependencies like this
There was a problem hiding this comment.
how was this ever merged? we played 48kHz at the wrong frequency?
There was a problem hiding this comment.
Unfortunately yes. maxim has a depenancy that BCLK should be exactly = FSYNC * CH * Bit till we had m/n divider BCLK should only be a divisior of MCLK and hence this was only working configuration available which maxim also suggested to be ok to use.
With m/n divider now available, we can satisfy the maxim requirement by giving 2.304 Mhz BCLK.
There was a problem hiding this comment.
so the problem I have here is that you have a strong dependency on firmware supporting M/N. There should be some sort of ABI dependency.
Well at the same time the initial code is completely broken, I sincerely hope no one ever used this topology...
There was a problem hiding this comment.
Earlier topology was only used for enabling purpose. Since the m/n divider is part of the new drop now, i pushed this change. Can we merge this so that we update the new topology ?
Is there a way we can protect this change under ABI dependancy - say if ABI > x then choose this config, else move to old config ?
There was a problem hiding this comment.
Is there a way we can protect this change under ABI dependancy - say if ABI > x then choose this config, else move to old config ?
@sathyap-chrome ABI info is generated at tplg build time using the script get_abi.sh and defined in the variables SOF_ABI_MAJOR/MINOR/PATCH in abi.h (this is generated so it will be in the build_tools/topology folder). You could include this in your topology m4 and check if the ABI version is valid to use the M/N divider.
|
> @@ -53,7 +53,7 @@ PCM_PLAYBACK_ADD(Speakers, 6, PIPELINE_PCM_8)
#SSP 1 (ID: 6)
DAI_CONFIG(SSP, 1, 6, SSP1-Codec,
SSP_CONFIG(I2S, SSP_CLOCK(mclk, 24000000, codec_mclk_in),
- SSP_CLOCK(bclk, 1500000, codec_slave),
- SSP_CLOCK(fsync, 46875, codec_slave),
Unfortunately yes. maxim has a depenancy that BCLK should be exactly =
FSYNC * CH * Bit till we had m/n divider BCLK should only be a divisior
of MCLK and hence this was only working configuration available which
maxim also suggested to be ok to use.
That configuration should never have been merged without a matching
sample-rate converter. Playing 48kHz at a different frequency is
unacceptable, regardless of what Maxim says.
With m/n divider now available, we can satisfy the maxim requirement by
giving 2.304 Mhz BCLK.
Right, but that should have been added first.
|
116beec to
a43f51b
Compare
a43f51b to
2e36a25
Compare
|
@sathyap-chrome does the latest code now have M/N support ? |
|
@lrgirdwo yes. |
lgirdwood
left a comment
lgirdwood
left a comment
There was a problem hiding this comment.
Btw, I'm assume M/N setting is implied from the MCLK/BCLK setting and not directly set in this file.
There was a problem hiding this comment.
Can we use 0xF here instead of F ? Makes it more obvious that it's a mask.
There was a problem hiding this comment.
Sure.
As per design, the M/N setting is automatically done based on the MCLK and BCLK and is not directed by the topology.
|
@sathyap-chrome Can you fix the build issues? |
|
Sure. i'm reworking the patches. sorry for typo in this one. |
Some of the features on topology need to have ABI version check. This adds few definitions to check ABI version being 3.9 or more. Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com> Signed-off-by: Sathya Prakash M R <sathya.prakash.m.r@intel.com>
Add rt1011 speaker CML Audio topology on top of sof_rt5682 where RT1011 is configured as TDM 4 SLOT ( 2 CH PB and 4 CH Feedback), 100 FS BCLK. BCLK delay is added for supported ABI versions. Signed-off-by: Sathya Prakash M R <sathya.prakash.m.r@intel.com>
2e36a25 to
b273710
Compare
sathyap-chrome
changed the title
With m/n divider support, we can now support 24 bit/ 48k on max98357a. Add check to only use update BCLK if m/n support is present. Else fallback to older settings. Signed-off-by: Sathya Prakash M R <sathya.prakash.m.r@intel.com>
|
Reworked the patches based on earlier suggestions. based on @ranj063 suggestion, we can consider both BCLK delay and m/n support added in ABI version 3.9. So version check is made as > 3.8 ( hence 3.9 or greater. |
ranj063
left a comment
ranj063
left a comment
There was a problem hiding this comment.
@sathyap-chrome the changes to check for ABI are fine but we need @plbossart to approve if the SSP config params are correct.
With m/n divider support, we can now support 24 bit/ 48k
on max98357a.
Signed-off-by: Sathya Prakash M R sathya.prakash.m.r@intel.com