SSP BLCK and MCLK early start - take 4 #4391
Conversation
bardliao
requested review from
a team,
dbaluta,
lbetlej,
lgirdwood and
mmaka1
as code owners
lgirdwood
left a comment
lgirdwood
left a comment
There was a problem hiding this comment.
@bardliao I can see an issue with APL no codec, but I suspect this is related to mixer. Can you add a subsequent patch here that uses a passthrough volume for PCM0 on APL nocodec topology. i.e remove the mixer pipeline and the SRC pipeline.
that's what I did already with PR #4378 |
|
@plbossart @bardliao I triggered a daily test level CI validation, the extra xrun still happen with this PR applied, please check the result of test plan 4820. |
|
XRUN also happens to ADLP_RVP_NOCODEC/multiple-pause-resume-25.sh which does not enable this feature. It seems to me the only difference in this case is the handling of SSCR1_TSRE/SSCR1_RSRE I'm wondering if it's valid to re-configure the SSP port for starting a streaming while the other one is in PAUSE state? That's what happen in this test case. |
it's impossible to draw any conclusion, this platform is already broken BEFORE testing this PR. We first need to have TGLU_NOCODEC to pass all tests before we can see what this impact of this PR might be. The same firmware/topology is used for ADL_NOCODEC in #4372 so the xruns/ipc timeouts need to be fixed first. Then we can see what this PR changes. |
I can see the xrun also happens in daily test 4778 multiple-pause-resume-25.sh case. Besides, simultaneous-playback-capture-25.sh and multiple-pause-resume-25.sh test are FAIL in daily test 4803, check-capture-50rounds.sh FAIL in daily test 4834. So I triggered a test (4841) with stable-v1.8 branch + this PR, but TGLU_RVP_NOCODEC even failed on the verify-kernel-boot-log.sh test. Maybe we should wait until TGLU_RVP_NOCODEC becomes stable and test again? |
This is definitely because of someone has changed to force dynamic pipelines on your DUT.
|
|
@bardliao can you rebase and add the BCLK_ES to sof-cavs_nocodec.m4, that replaces apl, cnl and tgl versions. Also not sure why you removed the 3.19 ABI change, is this because it's already been done. |
when the bclk is not based on the default source, we will thrown an error message but return the result of the mclk configuration. Fix by adding an explicit -EINVAL return value. Reported-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
The DAI_CONFIG IPC is currently used both for hw_params and hw_free. For some DAIs, there are hacky ways with e.g. invalid DMA channels to indicate a hw_free. Rather than adding a new IPC for hw_params and hw_free, let's add a flag that indicates if the DAI_CONFIG is really applied during a hw_params or hw_free stage. This is tagged as a ABI 3.19 change. Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Add two clks_control bits. MCLK and/or BCLK will start during hw_params and stop during hw_free if the corresponding bit is set. This is tagged as a ABI 3.19 change. Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
…e mclk/bclk MCLK and BCLK can be set/release separately. We can set/release mclk/bclk more flexible with these helpers. The helpers are defined after the Linux ones, 'prepare_enable' will first select the relevant resources then enable the clock output. Conversely 'disable_unprepare' will stop the clock output then release the clock resources. Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Some codecs need the SSP bit clock to start before data is provided, and conversely the bit clock to remain active until the hw_free stage. For backwards-compatibility with older kernels, the SOF_DAI_INTEL_SSP_CLKCTRL_MCLK_ES and SOF_DAI_INTEL_SSP_CLKCTRL_BCLK_ES bitfields are used to set an internal state in the ssp->clk_active field. This helps deals with the case where a topology sets these bits but the older kernel does not make use of the modified IPC. While we are at it, add clearer info traces for SSP configurations. Note that the FSYNC only starts when DMA transfers are enabled in the .trigger stage. This is by-design, the FSYNC will only start if the FIFO is not empty. During the prepare stages the DMA transfers are not enabled so the FIFOs are empty. To enable the FSYNC at an earlier stage, we would need a major surgery in the SOF architecture, or we would need to start zero-based DMA transfers. Co-developed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
The current code does not follow recommended programming sequences: TSRE and RSRE should be set before SSE and conversely cleared before SSE. Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Define the SSP_CC_MCLK/BCLK_ES bit to be used in SSP_CONFIG_DATA macro to enable mclk/bclk on hw_params and disable malk/bclk on hw_free. Signed-off-by: Brent Lu <brent.lu@intel.com> Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Set clks_control to (SOF_DAI_INTEL_SSP_CLKCTRL_MCLK_ES | SOF_DAI_INTEL_SSP_CLKCTRL_BCLK_ES) to enable MCLK/BCLK early start feature. Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Enable the bclk clock control for SSP2. Note that this impacts existing GLK-based chromebooks as well as newer hardware. Signed-off-by: Brent Lu <brent.lu@intel.com> Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
bardliao
force-pushed
the
bclk-early-start
branch
from
10352e2 to
32d04e9
Compare
@lgirdwood I didn't change the gain setting in this PR. And the same issue appears in recent daily tests. Like |
|
SOFCI TEST |
Thanks @bardliao I've checked today and I'm seeing this clipping on about ~50% of APL nocodec today so it's unrelated to your PR. Since this is loopback I'm suspicious the gain is possibly coming internal sampling (as levels will rise faster) of the MSB (as playback/capture with codec seems to be fine). @plbossart any comments, seems like this is not related to this PR. |
I don't know what's going on here, the updated test shows no issues with alsa-bat but problems with multi-pipelines and what looks like an xrun https://sof-ci.01.org/sofpr/PR4391/build9583/devicetest/?model=APL_UP2_NOCODEC&testcase=simultaneous-playback-capture I have this feeling that different runs are testing different configs. |
Yes, I think we are seeing lower frequency issues popping up here and unrelated to the changes in this PR. Could the multistream xruns get us into a state where we clip, not sure....but cant rule it out. |
plbossart
left a comment
plbossart
left a comment
There was a problem hiding this comment.
let's merge this so that we have a daily test with nocodec mode with all the latest changes
5 participants
The PR is an update of #4288. I didn't change anything except removing ABI: bump level to 3.19.