Start ssp clock earlier and stop later #4219

bardliao · 2021-05-20T07:02:15Z

Some codecs need ssp clock start earlier and stop later.

I didn't measure the clock yet, will do it soon.
@brentlu Can you test it on your side, too?

keyonjie · 2021-05-20T08:13:17Z

code looks good, maybe we can squash the 2nd and the 3rd commits together.

bardliao · 2021-05-20T11:32:21Z

This PR is alternative version of #4189

bardliao · 2021-05-20T11:37:30Z

code looks good, maybe we can squash the 2nd and the 3rd commits together.

Thanks @keyonjie I think separate the 2nd and the 3rd commits is easier to review, but I am fine to squash them, too. Lets keep it as it is for now and wait for comments from other reviewers.

lgirdwood · 2021-05-20T13:32:14Z

src/drivers/intel/ssp/ssp.c

Can we state what we are doing in comments at the start of each function. i.e. this function enabled clock x, y , z to support codec that need x, y z. This is called before triiger (x) and after trigger(y) etc

@bardliao @brentlu we probably need a topology token being passed to enable this operating mode (by default it should be disabled).

@bardliao we can use

/* SSP Configuration Request - SOF_IPC_DAI_SSP_CONFIG */ struct sof_ipc_dai_ssp_params { uint32_t reserved0; uint16_t reserved1; uint16_t mclk_id; uint32_t mclk_rate; /* mclk frequency in Hz */ uint32_t fsync_rate; /* fsync frequency in Hz */ uint32_t bclk_rate; /* bclk frequency in Hz */ /* TDM */ uint32_t tdm_slots; uint32_t rx_slots; uint32_t tx_slots; /* data */ uint32_t sample_valid_bits; uint16_t tdm_slot_width; uint16_t reserved2; /* alignment */ <<<< add a flag here, 1 means start BCKL early, 0 menas no change. /* MCLK */ uint32_t mclk_direction; uint16_t frame_pulse_width; uint16_t tdm_per_slot_padding_flag; uint32_t clks_control; uint32_t quirks; uint32_t bclk_delay; /* guaranteed time (ms) for which BCLK * will be driven, before sending data */ } __attribute__((packed, aligned(4)));

how about using quirks? it's just a flag to the ssp driver.

how about using quirks? it's just a flag to the ssp driver.

I don't know if it is a quirk. @lgirdwood what do you think?

Yeah, it's not a quirk but rather an operating mode. - We can use any of the reserved fields above as they are all 0 today and we could set a bit(s) for enabling the clock(s).

ranj063 · 2021-05-20T13:41:54Z

src/audio/dai.c

hw_free instead?

src/include/sof/lib/dai.h

bardliao · 2021-05-21T05:57:37Z

Updated, and the corresponding kernel PR is thesofproject/linux#2936

brentlu · 2021-05-23T06:52:08Z

src/drivers/intel/ssp/ssp.c

Could bclk continue to hw_free since SSE is clear here?

src/drivers/intel/ssp/ssp.c

brentlu · 2021-05-23T07:13:52Z

src/drivers/intel/ssp/ssp.c

since bclk_delay is added here, it seems to me that TSRE/RSRE bit is irrelevant to the clock itself.

since bclk_delay is added here, it seems to me that TSRE/RSRE bit is irrelevant to the clock itself.

I will check the spec, thanks.

Edit: This is for DMA transmit. I think it should be enabled after BCLK is enabled.

@bardliao have you checked if TSRE/RSRE set is mandatory/necessary for BCLK launching? or the setting of SSCR0_SSE? which is the crucial one?

@bardliao have you checked if TSRE/RSRE set is mandatory/necessary for BCLK launching? or the setting of SSCR0_SSE? which is the crucial one?

SSCR0_SSE is the crucial one.

When I set SSE bit, I see both bclk and fsync on my scope. @bardliao is it the same on your side?

We are told to enable TSRE and RSRE long before SSE, so that the fifos are filled.
That said, this is not really working because we don't seem to have a pipeline pre-roll so we still have a risk of FIFOs not being full before enabling the SSE bit. That will have to be another PR...

When I set SSE bit, I see both bclk and fsync on my scope. @bardliao is it the same on your side?

No, only bclk on my side. I tested on Up Xtreme (TGL) board. It looks like fsync starts when DMA starts.

We are told to enable TSRE and RSRE long before SSE, so that the fifos are filled.
That said, this is not really working because we don't seem to have a pipeline pre-roll so we still have a risk of FIFOs not being full before enabling the SSE bit. That will have to be another PR...

But look at the existing ssp_start we enable TSRE and RSRE after SSE.

I know @bardliao and hardware folks tell us it's wrong.
The trigger sequence needs to be revisited in a separate PR.

Sorry I am wrong, the SSE bit itself only enables bclk, not sure why they appear at the same time on the same device days before...

brentlu · 2021-05-23T07:38:58Z

src/include/kernel/tokens.h

Fail to cherry-pick but the ABI is already updated in 'ipc: dai-intel: add clk_early_start flag'...

No codespell typos will be found - file '/usr/share/codespell/dictionary.txt': No such file or directory
WARNING: Please update ABI in accordance with http://semver.org
#9: FILE: src/include/uapi/user/tokens.h:99:
#define SOF_TKN_INTEL_SSP_TDM_PADDING_PER_SLOT 505
+#define SOF_TKN_INTEL_SSP_CLK_EARLY_START 507

total: 0 errors, 1 warnings, 33 lines checked

Fail to cherry-pick but the ABI is already updated in 'ipc: dai-intel: add clk_early_start flag'...

No codespell typos will be found - file '/usr/share/codespell/dictionary.txt': No such file or directory
WARNING: Please update ABI in accordance with http://semver.org
#9: FILE: src/include/uapi/user/tokens.h:99:
#define SOF_TKN_INTEL_SSP_TDM_PADDING_PER_SLOT 505
+#define SOF_TKN_INTEL_SSP_CLK_EARLY_START 507

total: 0 errors, 1 warnings, 33 lines checked

Please cherry-pick from 2ce9552 to 806e766, thanks.

Does this token mean enabling all clocks i.e. MCLK, BCK and FRAME or just BCLK and FRAME ?
Also where do we set the duration of the "EARLY_START" ? or are we are meaning to enable the BCLK and FRAME as soon as we configure the DAI ? or at any other stage during PCM ops ?

Does this token mean enabling all clocks i.e. MCLK, BCK and FRAME or just BCLK and FRAME ?
Also where do we set the duration of the "EARLY_START" ? or are we are meaning to enable the BCLK and FRAME as soon as we configure the DAI ? or at any other stage during PCM ops ?

It means MCLK and BCLK. i.e. Doesn't affect FRAME. With this token enabled, MCLK/BCLK will start during hw_parames and stop during hw_free.

plbossart

thanks @bardliao for starting this. I added a set of comments below, the most important is that on stop we have to check for both directions, ie. we start the clock once and stop it when both directions are stopped.

plbossart · 2021-05-24T13:44:44Z

src/include/sof/lib/dai.h

I suggest listing the callback by groups and in the order in which they are used. I found out by reverse-engineering the code that set_config is actually called during the prepare stage after the hw_params, and it's clearly not evident from the list.

I suggest listing the callback by groups and in the order in which they are used. I found out by reverse-engineering the code that set_config is actually called during the prepare stage after the hw_params, and it's clearly not evident from the list.

I have to admit that I don't know the order the callback are used. Maybe someone can create another PR to do it.

well at the minimum you need to make sure the .hw_params is called after set_config, otherwise the clocks will be set with values that haven't been initialized yet. Unfortunately in my experiments this was the other way around, set_config was called in the prepare stage after hw_params.

@keyonjie and @lgirdwood we really need your help to clarify what the dai_driver state machine looks like and what callbacks are used in what order.

@plbossart set_config is called before hw_params for sure both in the case of static pipelines (during tplg parsing and during BE hw_params) and dynamic pipelines (only during BE hw_params)

@ranj063 my traces show otherwise. I think there is a difference between the IPC DAI_CONFIG and the set_config callback in the dai_driver. See the code, the .set_config callback is used in the .prepare stage.

@plbossart From my trace log, set_config is called before hw_params. Look into the code, dai_set_config is called when we send a IPC with SOF_IPC_DAI_CONFIG cmd, which is in sof_link_load. In short, set_config is called during topology loading stage and hw_params is called during pcm open stage. So set_config should be called before hw_params.

plbossart · 2021-05-24T13:45:40Z

src/include/sof/lib/dai.h

do we need to return an integer if we don't test for errors in the caller?

src/drivers/intel/ssp/ssp.c

plbossart · 2021-05-24T13:49:06Z

src/drivers/intel/ssp/ssp.c

I think we should enable MCLK and BCLK separately. They are not logically connected.

plbossart · 2021-05-24T13:51:50Z

src/drivers/intel/ssp/ssp.c

We probably need a change to the code in https://github.com/thesofproject/sof/blob/c7b75f536e9fcbaba6d647bfe02413a572be82c0/src/drivers/intel/ssp/ssp.c#L632, the component does not become active here so if we call ssp_pre_start a second time the protection does not work

plbossart · 2021-05-24T13:53:27Z

src/drivers/intel/ssp/ssp.c

typo: trigger

plbossart · 2021-05-24T13:55:15Z

src/drivers/intel/ssp/ssp.c

don't we need to check for the direction to mirror what happens line 876? if the playback and capture are started separately we need to stop the SSP when both are stopped.

why do we need this here again? we're doing this already in hw_params() too

jgunn0262 · 2021-05-25T10:41:33Z

@bardliao I think this is a good feature. I've worked on codecs in the past that would need the BCLK & FRAME clock enabled to do any I2C IO. Would this change also support such codecs ?

bardliao · 2021-05-25T11:20:12Z

thanks @bardliao for starting this. I added a set of comments below, the most important is that on stop we have to check for both directions, ie. we start the clock once and stop it when both directions are stopped.

Thanks @plbossart All comments are addressed.

bardliao · 2021-05-25T11:24:06Z

@bardliao I think this is a good feature. I've worked on codecs in the past that would need the BCLK & FRAME clock enabled to do any I2C IO. Would this change also support such codecs ?

Yes, but this PR doesn't change FRAME clock enabled time. I wasn't aware any codec need FRAME clock to do I2C IO. We may need to add a new clks_control bit to handle such case.

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. Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>

So that we can stop ssp clock in dai_reset. Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>

plbossart

the state management needs a bit more work I think, it's not clear why we use ACTIVE and PREPARE in similar steps.

plbossart · 2021-05-25T15:48:50Z

src/drivers/intel/ssp/ssp.c

I don't think that test is valid for the early start. the ACTVE state is set in the trigger case.

I don't think that test is valid for the early start. the ACTVE state is set in the trigger case.

ssp_pre_start() is only called by ssp_start() now, so it is not used for the early start.

plbossart · 2021-05-25T15:51:49Z

src/drivers/intel/ssp/ssp.c

We should have some sort of symmetry in the function names between bclk_set and release_bclk? get/put, acquire/release, etc.

We should have some sort of symmetry in the function names between bclk_set and release_bclk? get/put, acquire/release, etc.

added ssp_release_bclk and ssp_release_mclk

plbossart · 2021-05-25T15:54:27Z

src/drivers/intel/ssp/ssp.c

why use == PREPARE and != ACTIVE is the two different bclk and mclk cases? the state management isn't clear to me.

why use == PREPARE and != ACTIVE is the two different bclk and mclk cases? the state management isn't clear to me.

That was copied from ssp_post_stop(). I will change it to == COMP_STATE_PREPARE. I also thing using == COMP_STATE_ACTIVE to test if clock is already active is not right. The clock will keep active when ssp->state = COMP_STATE_PAUSED. @keyonjie what do you think?

why use == PREPARE and != ACTIVE is the two different bclk and mclk cases? the state management isn't clear to me.

That was copied from ssp_post_stop(). I will change it to == COMP_STATE_PREPARE. I also thing using == COMP_STATE_ACTIVE to test if clock is already active is not right. The clock will keep active when ssp->state = COMP_STATE_PAUSED. @keyonjie what do you think?

yes, we can't use the logic of pre_start/post_stop directly here, but we do need check here to avoid set/release clocks when not needed, imagine that when the playback has already enabled mclk/bclk, we should not try to enable them again when a arecord on the same SSP port arrives.

plbossart · 2021-05-25T15:55:42Z

src/drivers/intel/ssp/ssp.c

have you checked that the hw_params is called after set_config? if not, the values will be uninitialized.

ranj063 · 2021-05-25T16:19:30Z

src/drivers/intel/ssp/ssp.c

no need to init?

no need to init?

We need to init because ret = mn_set_bclk() is only called if CONFIG_INTEL_MN

there's something weird there: in the #else case below if the condition isn't satisfied we print an error message and jump to out, but we don't set ret... Is that correct?

ranj063 · 2021-05-25T16:20:12Z

src/drivers/intel/ssp/ssp.c

return ssp_set_bclk(dai)?

Maybe just remove if (ret < 0) return ret; below

ranj063 · 2021-05-25T16:28:37Z

src/drivers/intel/ssp/ssp.c

why do we need this here again? we're doing this already in hw_params() too

ranj063 · 2021-05-25T16:31:54Z

src/drivers/intel/ssp/ssp.c

Im getting super confused here. We do the same things in ssp_hw_params() and ssp_pre_start(). Why do we need to do these twice?

Im getting super confused here. We do the same things in ssp_hw_params() and ssp_pre_start(). Why do we need to do these twice?

They are not called twice. We do them in ssp_hw_params() if SOF_DAI_INTEL_SSP_CLKCTRL_BCLK_ES is set, and do them in ssp_pre_start() if SOF_DAI_INTEL_SSP_CLKCTRL_BCLK_ES is not set. And same thing as MCLK.

plbossart · 2021-05-25T17:35:10Z

@bardliao I think this is a good feature. I've worked on codecs in the past that would need the BCLK & FRAME clock enabled to do any I2C IO. Would this change also support such codecs ?

Yes, but this PR doesn't change FRAME clock enabled time. I wasn't aware any codec need FRAME clock to do I2C IO. We may need to add a new clks_control bit to handle such case.

When I checked with our hardware folks on the SSP capabilities, the answer was that the BLCK and FSYNC start at the same time when you set the SSE bit. We clearly do not have the ability to start e.g. FSYNC without BCLK also being enabled.

That said, I've see weird things and delays in my experiments so I am not completely sure of the dependencies between the two types of clocks.

We will enable/disable ssp clock in the ops. Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>

Some codecs need ssp clock to start before codec is initialized and stop after codec is down. Use SOF_DAI_INTEL_SSP_CLKCTRL_MCLK_ES and SOF_DAI_INTEL_SSP_CLKCTRL_BCLK_ES bits to start MCLK/BCLK during hw_parames and stop during hw_free. Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>

bardliao · 2021-05-27T02:14:20Z

I like the idea of having a separate flag to control the SSP state @bardliao this is good.

I think this is missing a change in a couple of topology files to set those bits and to actually check in CI and manual tests that the result works. We should e.g. change the apl_nocodec and glk topologies to use this by default.

@plbossart I upload some topologies change, but I don't know if the change of nocodec should go into upstream or not.

bardliao · 2021-05-27T02:29:00Z

@bardliao @brentlu, does this PR work in all cases for you?

I added this patch to test it
diff --git a/src/drivers/intel/ssp/ssp.c b/src/drivers/intel/ssp/ssp.c
index 0e27ad26e..f2e1e8d28 100644
--- a/src/drivers/intel/ssp/ssp.c
+++ b/src/drivers/intel/ssp/ssp.c
@@ -254,6 +254,9 @@ static int ssp_set_config(struct dai *dai, struct ipc_config_dai *common_config,
                goto out;
        }
 
+       ssp->params.clks_control |= SOF_DAI_INTEL_SSP_CLKCTRL_MCLK_ES;
+       ssp->params.clks_control |= SOF_DAI_INTEL_SSP_CLKCTRL_BCLK_ES;
+
        /* supporting bclk idle state */
        if (ssp->params.clks_control &
                SOF_DAI_INTEL_SSP_CLKCTRL_BCLK_IDLE_HIGH) {
and the results are ok on stop, with an 18ms delay between stopping the data and stopping the clocks

but on start I see no difference, still the same 4.2ms data delay, and actually the SSP FSYNC is delayed somehow. This doesn't seem to align with the notion that FSYNC and BCLK should start at the same time.

Edit: on Stop I see a 18ms delay even without enabling those bits, so this PR seems to add a problem on start but not improve anything on stop.

@brentlu can you share your scope traces?

@plbossart I tested it with below change on kernel side. So that I can check the clock easier. I do see the PR take effect on both start and stop. But MCLK always stop when DSP is off with or without this PR.

diff --git a/sound/soc/sof/pcm.c b/sound/soc/sof/pcm.c
index c9ea05c643c3..6471b8a27bcc 100644
--- a/sound/soc/sof/pcm.c
+++ b/sound/soc/sof/pcm.c
@@ -268,6 +268,9 @@ static int sof_pcm_hw_params(struct snd_soc_component *component,
                return ret;
        }

+       pr_err("bard: %s sleep start\n", __func__);
+       msleep(2000);
+       pr_err("bard: %s sleep end\n", __func__);
        ret = sof_pcm_dsp_params(spcm, substream, &ipc_params_reply);
        if (ret < 0)
                return ret;
@@ -467,6 +470,9 @@ static int sof_pcm_trigger(struct snd_soc_component *component,

        /* free PCM if reset_hw_params is set and the STOP IPC is successful */
        if (!ret && reset_hw_params) {
+               pr_err("bard: %s sleep start\n", __func__);
+               msleep(5000);
+               pr_err("bard: %s sleep end\n", __func__);
                ret = sof_pcm_dsp_pcm_free(substream, sdev, spcm);
                if (ret < 0)
                        return ret;

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 contrl for SSP2. The 10ms bclk delay is no longer needed since bclk is enabled eariler on hw_params. Signed-off-by: Brent Lu <brent.lu@intel.com> Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>

plbossart · 2021-05-27T16:48:07Z

@bardliao I don't know what I could be doing wrong, but I tried this PR multiple times and I don't see a difference on UP2-nocodec. the bclk and fsync start 4.2ms before the data and stop 18ms later.

I suspect a configuration issue since the kernel patch above does not apply on any of my branches.

Can you please state which SHA1s you are using for firmware/topology and kernel?

plbossart · 2021-05-27T16:53:13Z

Answering to my own question, it seems the clock control is not properly setup on Up2-nocodec: clks_control 0x40 means only the MCLK is set (BIT6). BCLK is not set, wondering if this is an m4 issue

[       29912.498811] (           4.895833) c0 pipe         11.45 ....../pipeline-params.c:215  pipe params dir 0 frame_fmt 0 buffer_fmt 0 rate 48000
[       29917.707145] (           5.208333) c0 pipe         11.45 ....../pipeline-params.c:219  pipe params stream_tag 1 channels 2 sample_valid_bytes 2 sample_container_bytes 2
[       29961.352976] (          43.645832) c0 dai          11.44       src/ipc/dai-ipc3.c:84   dai_data_config() dai type = 1 index = 5 dd 0xbe04cb80
[       29969.113392] (           7.760417) c0 ssp-dai      1.5   /drivers/intel/ssp/ssp.c:825  ssp_hw_params() params.clks_control 0x40
[       29976.457142] (           7.343750) c0 mn                 ./drivers/intel/ssp/mn.c:258  mclk_rate 24576000, mclk_source_clock 1
[       29982.082142] (           5.625000) c0 mn                 ./drivers/intel/ssp/mn.c:220  mclk_id 0 mdivr_val 1
[       30003.905058] (          21.822916) c0 dai          11.44          src/audio/dai.c:346  dai_playback_params() dest_dev = 12 stream_id = 0 src_width = 2 dest_width = 2
[       30009.946724] (           6.041667) c0 dai          11.44          src/audio/dai.c:352  dai_playback_params() fifo 0x8a10
[       30021.040474] (          11.093750) c0 pipe         11.45 ....../pipeline-params.c:297  pipe prepare
[       30048.123806] (          27.083332) c0 dai          11.44          src/audio/dai.c:639  dai_prepare()
[       30054.686306] (           6.562500) c0 dai          11.44          src/audio/dai.c:586  dai_config_prepare(), channel = 0
[       30060.415472] (           5.729167) c0 dw-dma                 src/drivers/dw/dma.c:191  dw_dma_channel_get(): dma 0 request channel 0
[       30066.873805] (           6.458333) c0 dai          11.44          src/audio/dai.c:603  dai_config(): new configured dma channel index 0
[      500399.875949] (      470333.000000) c0 ll-schedule        ./schedule/ll_schedule.c:224  perf ll_work peak plat 915 cpu 15222
[     1121888.288753] (      621488.437500) c0 pipe         11.45 ....../pipeline-stream.c:184  pipe trigger cmd 1
[     1121943.497085] (          55.208332) c0 ssp-dai      1.5   /drivers/intel/ssp/ssp.c:984  ssp_trigger() cmd 1
[     1121949.747084] (           6.250000) c0 ssp-dai      1.5   /drivers/intel/ssp/ssp.c:731  ssp_pre_start()
[     1121957.351251] (           7.604167) c0 mn                 ./drivers/intel/ssp/mn.c:320  find_mn for freq 24576000 bclk 1536000
[     1121963.757500] (           6.406250) c0 mn                 ./drivers/intel/ssp/mn.c:617  bclk_rate 1536000, *out_scr_div 16, m 1, n 1
[     1121970.163750] (           6.406250) c0 ssp-dai      1.5   /drivers/intel/ssp/ssp.c:701  ssp_set_config(), sscr0 = 0xc1c00f7f
[     1121976.622083] (           6.458333) c0 ssp-dai      1.5   /drivers/intel/ssp/ssp.c:898  ssp_start()
[     1122029.799165] (          53.177082) c0 ll-schedule        ./schedule/ll_schedule.c:406  task add 0xbe049100 pipe-task <f11818eb-e92e-4082-82a3-dc54c604ebb3>
[     1122036.205414] (           6.406250) c0 ll-schedule        ./schedule/ll_schedule.c:410  task params pri 0 flags 0 start 0 period 1000
[     1122047.090831] (          10.885416) c0 ll-schedule        ./schedule/ll_schedule.c:322  new added task->start 22262173 at 22256430
[     1122053.028330] (           5.937500) c0 ll-schedule        ./schedule/ll_schedule.c:325  num_tasks 3 total_num_tasks 3
[     1122358.757485] (         305.729156) c0 sa                          src/lib/agent.c:65   perf sys_load peak plat 19229 cpu 320494
[     1122380.840817] (          22.083332) c0 host         11.40 ....../pipeline-stream.c:204  perf comp_copy peak plat 233 cpu 3892
[     1122420.111649] (          39.270832) c0 pga          11.41 ....../pipeline-stream.c:204  perf comp_copy peak plat 602 cpu 10002
[     1122436.726232] (          16.614582) c0 dai          11.44 ....../pipeline-stream.c:204  perf comp_copy peak plat 163 cpu 2742
[     1122478.340813] (          41.614582) c0 ll-schedule        ./schedule/ll_schedule.c:224  perf ll_work peak plat 2465 cpu 41006
[     2143856.841894] (     1021378.500000) c0 pipe         11.45 ....../pipeline-stream.c:184  pipe trigger cmd 0
[     2143874.498143] (          17.656250) c0 ssp-dai      1.5   /drivers/intel/ssp/ssp.c:984  ssp_trigger() cmd 0
[     2144872.414770] (         997.916626) c0 wait                         src/lib/wait.c:45   ERROR ewt
[     2144877.935604] (           5.520833) c0 ssp-dai      1.5   /drivers/intel/ssp/ssp.c:58   WARN ssp_empty_tx_fifo() warning: timeout
[     2144884.341853] (           6.406250) c0 ssp-dai      1.5   /drivers/intel/ssp/ssp.c:950  ssp_stop(), TX stop
[     2144890.279353] (           5.937500) c0 ssp-dai      1.5   /drivers/intel/ssp/ssp.c:959  ssp_stop(), SSP port disabled
[     2144896.060603] (           5.781250) c0 ssp-dai      1.5   /drivers/intel/ssp/ssp.c:766  ssp_post_stop releasing BCLK clocks for SSP5...
[     2144903.716853] (           7.656250) c0 dw-dma                 src/drivers/dw/dma.c:407  dw_dma_stop(): dma 0 channel 0 stop
[     2144913.091852] (           9.375000) c0 ll-schedule        ./schedule/ll_schedule.c:531  task cancel 0xbe049100 pipe-task <f11818eb-e92e-4082-82a3-dc54c604ebb3>
[     2144920.175185] (           7.083333) c0 ll-schedule        ./schedule/ll_schedule.c:352  num_tasks 2 total_num_tasks 2
[     3207459.403797] (     1062539.250000) c0 pipe         11.45 ......./pipeline-graph.c:383  pipe reset
[     3207494.091296] (          34.687500) c0 dai          11.44          src/audio/dai.c:688  dai_reset()
[     3207500.132962] (           6.041667) c0 dw-dma                 src/drivers/dw/dma.c:254  dw_dma_channel_put(): dma 0 channel 0 put
[     3207517.216295] (          17.083332) c0 ssp-dai      1.5   /drivers/intel/ssp/ssp.c:858  ssp_hw_free()
[     3207522.997544] (           5.781250) c0 ssp-dai      1.5   /drivers/intel/ssp/ssp.c:873  ssp_hw_free releasing MCLK clocks for SSP5...
[     3207878.622530] (         355.625000) c0 ipc                  src/ipc/handler-ipc3.c:1284 ipc: comp 40 -> free
[     3207884.924613] (           6.302083) c0 host         11.40         src/audio/host.c:577  host_free()
[     3207890.966280] (           6.041667) c0 hda-dma            ..../intel/hda/hda-dma.c:849  hda-dmac :5 -> remove
[     3207917.789195] (          26.822916) c0 dma                           src/lib/dma.c:141  dma_put(), dma = 0x9e042a40, sref = 0
[     3208167.424602] (         249.635406) c0 ipc                  src/ipc/handler-ipc3.c:1284 ipc: comp 42 -> free
[     3208395.289176] (         227.864578) c0 ipc                  src/ipc/handler-ipc3.c:1284 ipc: comp 41 -> free
[     3208604.247502] (         208.958328) c0 ipc                  src/ipc/handler-ipc3.c:1284 ipc: comp 43 -> free
[     3208820.549576] (         216.302078) c0 ipc                  src/ipc/handler-ipc3.c:1284 ipc: comp 45 -> free
[     3208826.487076] (           5.937500) c0 pipe         11.45 ......./pipeline-graph.c:224  pipeline_free()
[     3208848.882908] (          22.395832) c0 memory                      src/lib/alloc.c:1083 heap: buffer status
[     3208854.403742] (           5.520833) c0 memory                      src/lib/alloc.c:1057  heap: 0xbe058280 size 65536 blocks 1 caps 0x71
[     3208860.080825] (           5.677083) c0 memory                      src/lib/alloc.c:1059   used 17152 free 48384
[     3208865.966241] (           5.885417) c0 memory                      src/lib/alloc.c:1057  heap: 0xbe800000 size 131072 blocks 1 caps 0x69
[     3208871.591241] (           5.625000) c0 memory                      src/lib/alloc.c:1059   used 0 free 131072
[     3208876.955824] (           5.364583) c0 memory                      src/lib/alloc.c:1085 heap: runtime status
[     3208882.424574] (           5.468750) c0 memory                      src/lib/alloc.c:1057  heap: 0xbe048a80 size 63488 blocks 7 caps 0x45
[     3208888.153740] (           5.729167) c0 memory                      src/lib/alloc.c:1059   used 3200 free 60288
[     3208893.622490] (           5.468750) c0 memory                      src/lib/alloc.c:1067   block 1 base 0xbe04aa80 size 128
[     3208899.143323] (           5.520833) c0 memory                      src/lib/alloc.c:1070    count 64 free 64
[     3208904.559990] (           5.416667) c0 memory                      src/lib/alloc.c:1067   block 2 base 0xbe04ca80 size 256
[     3208910.080823] (           5.520833) c0 memory                      src/lib/alloc.c:1070    count 128 free 125
[     3208915.497489] (           5.416667) c0 memory                      src/lib/alloc.c:1067   block 3 base 0xbe054a80 size 512
[     3208921.018322] (           5.520833) c0 memory                      src/lib/alloc.c:1070    count 8 free 5
[     3208926.487072] (           5.468750) c0 memory                      src/lib/alloc.c:1067   block 4 base 0xbe055a80 size 1024
[     3208932.007905] (           5.520833) c0 memory                      src/lib/alloc.c:1070    count 4 free 4
[     3209204.716228] (         272.708313) c0 dma-trace                 src/trace/trace.c:133  Suppressed 2 similar messages:   block %d base 0x%x size %d
[     3209209.039144] (           4.322917) c0 dma-trace                 src/trace/trace.c:133  Suppressed 2 similar messages:    count %d free %d
[     3209213.674561] (           4.635417) c0 ipc                  src/ipc/handler-ipc3.c:547  ipc: dai 1.5 -> config 
[     3209220.341227] (           6.666667) c0 dai                           src/lib/dai.c:164  dai_get type 1 index 5 new sref 2
[     3209226.955810] (           6.614583) c0 ssp-dai      1.5   /drivers/intel/ssp/ssp.c:173  ssp_set_config(), config->format = 0x4001
[     3209236.278726] (           9.322916) c0 ssp-dai      1.5   /drivers/intel/ssp/ssp.c:600  ssp_set_config(), sscr0 = 0xc1c0003f, sscr1 = 0xd0700004, ssto = 0x00000000, sspsp = 0x2100000
[     3209242.164143] (           5.885417) c0 ssp-dai      1.5   /drivers/intel/ssp/ssp.c:602  ssp_set_config(), sscr2 = 0x00004002, sspsp2 = 0x00000000, sscr3 = 0x03030000, ssioc = 0x00000020
[     3209247.684976] (           5.520833) c0 ssp-dai      1.5   /drivers/intel/ssp/ssp.c:604  ssp_set_config(), ssrsa = 0x00000003, sstsa = 0x00000003
[     3209254.091226] (           6.406250) c0 dai                           src/lib/dai.c:188  dai_put type 1 index 5 new sref 1
[     3209260.132892] (           6.041667) c0 ipc                      src/ipc/dai-ipc3.c:159  ipc_comp_dai_config() dai type = 1 index = 5
[     3209266.174559] (           6.041667) c0 dai          11.44       src/ipc/dai-ipc3.c:223  dai_config() dai type = 1 index = 5 dd 0xbe04cb80
[     3209451.903718] (         185.729156) c0 ipc                  src/ipc/handler-ipc3.c:1284 ipc: comp 44 -> free
[     3209460.028718] (           8.125000) c0 power              ../cavs/lib/pm_runtime.c:296  en-dwdma-clk-gating index 0 CLKCTL 00080002
[     3209494.664133] (          34.635414) c0 dma                           src/lib/dma.c:141  dma_put(), dma = 0x9e042980, sref = 0
[     3209501.226633] (           6.562500) c0 power              ../cavs/lib/pm_runtime.c:161  en-ssp-clk-gating index 5 CLKCTL 00000002
[     3209511.591215] (          10.364583) c0 dai                           src/lib/dai.c:188  dai_put type 1 index 5 new sref 0

plbossart · 2021-05-27T17:23:05Z

@bardliao I can confirm that the nocodec M4 stuff does not work with the MCLK | BCLK case. Using BCLK_ES only gives me the right config with BIT(7) set.

That said, when I add you kernel patch to add a 1s delay on startup, I see it but on bclk only. Somehow the FSYNC starts with the data, that's not aligned with what the documentation says.

Can you confirm if you've seen the FSYNC also toggle earlier than the data and start at the same time as BCLK?

plbossart · 2021-05-27T17:26:29Z

@bardliao see below the patch I used to see something in nocodec mode. Maybe this is missing an 'eval' for M4 to do the bitwise OR. @ranj063 can you help?

diff --git a/tools/topology/sof-apl-nocodec.m4 b/tools/topology/sof-apl-nocodec.m4
index e32bc753b..e8de3731f 100644
--- a/tools/topology/sof-apl-nocodec.m4
+++ b/tools/topology/sof-apl-nocodec.m4
@@ -273,7 +273,7 @@ DAI_CONFIG(SSP, 0, 0, NoCodec-0,
                      SSP_TDM(2, 16, 3, 3),
                      dnl SSP_CONFIG_DATA(type, dai_index, valid bits, mclk_id, quirks)
                      SSP_CONFIG_DATA(SSP, 0, 16, 0, SSP_QUIRK_LBM, 0,
-                                     SSP_CC_MCLK_ES | SSP_CC_BCLK_ES)))
+                                     SSP_CC_BCLK_ES)))
 
 DAI_CONFIG(SSP, 1, 1, NoCodec-1,
           SSP_CONFIG(I2S, SSP_CLOCK(mclk, 24576000, codec_mclk_in),
@@ -281,7 +281,7 @@ DAI_CONFIG(SSP, 1, 1, NoCodec-1,
                      SSP_CLOCK(fsync, 48000, codec_slave),
                      SSP_TDM(2, 16, 3, 3),
                      SSP_CONFIG_DATA(SSP, 1, 16, 0, SSP_QUIRK_LBM, 0,
-                                     SSP_CC_MCLK_ES | SSP_CC_BCLK_ES)))
+                                     SSP_CC_BCLK_ES)))
 
 #DAI_CONFIG(SSP, 2, 2, NoCodec-2,
 #         SSP_CONFIG(I2S, SSP_CLOCK(mclk, 24576000, codec_mclk_in),
@@ -289,7 +289,7 @@ DAI_CONFIG(SSP, 1, 1, NoCodec-1,
 #                    SSP_CLOCK(fsync, 48000, codec_slave),
 #                    SSP_TDM(2, 16, 3, 3),
 #                    SSP_CONFIG_DATA(SSP, 2, 16, 0, SSP_QUIRK_LBM, 0,
-#                                    SSP_CC_MCLK_ES | SSP_CC_BCLK_ES)))
+#                                    SSP_CC_BCLK_ES)))
 
 DAI_CONFIG(SSP, 3, 3, NoCodec-3,
           SSP_CONFIG(I2S, SSP_CLOCK(mclk, 24576000, codec_mclk_in),
@@ -297,7 +297,7 @@ DAI_CONFIG(SSP, 3, 3, NoCodec-3,
                      SSP_CLOCK(fsync, 48000, codec_slave),
                      SSP_TDM(2, 16, 3, 3),
                      SSP_CONFIG_DATA(SSP, 3, 16, 0, SSP_QUIRK_LBM, 0,
-                                     SSP_CC_MCLK_ES | SSP_CC_BCLK_ES)))
+                                     SSP_CC_BCLK_ES)))
 
 #DAI_CONFIG(SSP, 4, 4, NoCodec-4,
 #         SSP_CONFIG(I2S, SSP_CLOCK(mclk, 24576000, codec_mclk_in),
@@ -305,7 +305,7 @@ DAI_CONFIG(SSP, 3, 3, NoCodec-3,
 #                    SSP_CLOCK(fsync, 48000, codec_slave),
 #                    SSP_TDM(2, 16, 3, 3),
 #                    SSP_CONFIG_DATA(SSP, 4, 16, 0, SSP_QUIRK_LBM, 0,
-#                                    SSP_CC_MCLK_ES | SSP_CC_BCLK_ES)))
+#                                    SSP_CC_BCLK_ES)))
 
 DAI_CONFIG(SSP, 5, 5, NoCodec-5,
           SSP_CONFIG(I2S, SSP_CLOCK(mclk, 24576000, codec_mclk_in),
@@ -313,7 +313,7 @@ DAI_CONFIG(SSP, 5, 5, NoCodec-5,
                      SSP_CLOCK(fsync, 48000, codec_slave),
                      SSP_TDM(2, 16, 3, 3),
                      SSP_CONFIG_DATA(SSP, 5, 16, 0, SSP_QUIRK_LBM, 0,
-                                     SSP_CC_MCLK_ES | SSP_CC_BCLK_ES)))
+                                     SSP_CC_BCLK_ES)))
 
 DAI_CONFIG(DMIC, 0, 6, NoCodec-6,
           dnl DMIC_CONFIG(driver_version, clk_min, clk_mac, duty_min, duty_max,

ranj063 · 2021-05-27T17:34:52Z

@bardliao see below the patch I used to see something in nocodec mode. Maybe this is missing an 'eval' for M4 to do the bitwise OR. @ranj063 can you help?

@plbossart can you please try this:

SSP_CONFIG_DATA(SSP, 4, 16, 0, SSP_QUIRK_LBM, 0,
                                    eval(`SSP_CC_MCLK_ES + SSP_CC_BCLK_ES')

plbossart · 2021-05-27T18:28:28Z

@bardliao see below the patch I used to see something in nocodec mode. Maybe this is missing an 'eval' for M4 to do the bitwise OR. @ranj063 can you help?

@plbossart can you please try this:
SSP_CONFIG_DATA(SSP, 4, 16, 0, SSP_QUIRK_LBM, 0,
                                    eval(`SSP_CC_MCLK_ES + SSP_CC_BCLK_ES')

Doesn't M4 support bitwise OR? https://www.gnu.org/software/m4/manual/m4-1.4.15/html_node/Eval.html#Eval

ranj063 · 2021-05-27T18:29:20Z

@bardliao see below the patch I used to see something in nocodec mode. Maybe this is missing an 'eval' for M4 to do the bitwise OR. @ranj063 can you help?

@plbossart can you please try this:
SSP_CONFIG_DATA(SSP, 4, 16, 0, SSP_QUIRK_LBM, 0,
                                    eval(`SSP_CC_MCLK_ES + SSP_CC_BCLK_ES')
Doesn't M4 support bitwise OR? https://www.gnu.org/software/m4/manual/m4-1.4.15/html_node/Eval.html#Eval

yeah it does but the end result is the same in this case right?

brentlu · 2021-05-28T07:01:19Z

There is a problem found. On Chrome-v4.14, our hw_free is called from the CPU DAI of FE DAI Link which is earlier than codec driver's call-back which is on CODEC DAI of BE DAI Link.

[ 69.321714] sound pcmC0D1p: snd_pcm_common_ioctl: DRAIN
[ 69.662699] sof-audio-pci 0000:00:0e.0: ipc tx: 0x60050000: GLB_STREAM_MSG: TRIG_STOP
[ 69.663229] sof-audio-pci 0000:00:0e.0: ipc tx succeeded: 0x60050000: GLB_STREAM_MSG: TRIG_STOP
[ 69.663268] sound pcmC0D1p: snd_pcm_common_ioctl: DROP
[ 69.663273] sound pcmC0D1p: snd_pcm_common_ioctl: HW_FREE
=> Hw_free here
[ 69.663279] sof-audio-pci 0000:00:0e.0: ipc tx: 0x60030000: GLB_STREAM_MSG: PCM_FREE
[ 69.663536] sof-audio-pci 0000:00:0e.0: ipc tx succeeded: 0x60030000: GLB_STREAM_MSG: PCM_FREE
=> Bclk disabled here

[ 69.663541] cs42l42 i2c-10134242:00: cs42l42_mute_stream:
[ 69.676599] cs42l42 i2c-10134242:00: cs42l42_pcm_hw_free:
=> codec's hw free here:
[ 69.676668] sound pcmC0D1p: snd_pcm_release:
[ 70.580361] sof-audio-pci 0000:00:0e.0: ipc rx: 0x90020000: GLB_TRACE_MSG
[ 70.580394] sof-audio-pci 0000:00:0e.0: ipc rx done: 0x90020000: GLB_TRACE_MSG
[ 71.736169] sof-audio-pci 0000:00:0e.0: ipc tx: 0x40010000: GLB_PM_MSG: CTX_SAVE
[ 71.736399] sof-audio-pci 0000:00:0e.0: ipc tx succeeded: 0x40010000: GLB_PM_MSG: CTX_SAVE

On SOF Linux, I found the PCM_FREE command is sent even earlier...

[ 256.423305] sound pcmC0D0p: snd_pcm_common_ioctl: DROP
[ 256.425620] sof-audio-pci-intel-tgl 0000:00:1f.3: ipc tx: 0x60050000: GLB_STREAM_MSG: TRIG_STOP
[ 256.425916] sof-audio-pci-intel-tgl 0000:00:1f.3: ipc tx succeeded: 0x60050000: GLB_STREAM_MSG: TRIG_STOP
[ 256.425939] sof-audio-pci-intel-tgl 0000:00:1f.3: ipc tx: 0x60030000: GLB_STREAM_MSG: PCM_FREE
[ 256.426271] sof-audio-pci-intel-tgl 0000:00:1f.3: ipc tx succeeded: 0x60030000: GLB_STREAM_MSG: PCM_FREE
[ 256.426361] sound pcmC0D0p: snd_pcm_common_ioctl: HW_FREE
[ 256.428063] sof-audio-pci-intel-tgl 0000:00:1f.3: ipc tx: 0x80010000: GLB_DAI_MSG: CONFIG
[ 256.428429] sof-audio-pci-intel-tgl 0000:00:1f.3: ipc tx succeeded: 0x80010000: GLB_DAI_MSG: CONFIG
[ 256.428530] sound pcmC0D0p: snd_pcm_release:
[ 257.097936] sof-audio-pci-intel-tgl 0000:00:1f.3: ipc rx: 0x90020000: GLB_TRACE_MSG
[ 257.097975] sof-audio-pci-intel-tgl 0000:00:1f.3: ipc rx done: 0x90020000: GLB_TRACE_MSG
[ 260.152371] sof-audio-pci-intel-tgl 0000:00:1f.3: ipc tx: 0x40010000: GLB_PM_MSG: CTX_SAVE
[ 260.152609] sof-audio-pci-intel-tgl 0000:00:1f.3: ipc tx succeeded: 0x40010000: GLB_PM_MSG: CTX_SAVE

I add a DAI_HW_FREE ipc command in BE DAI's hw_free(). It still comes before codec dai's hw_free() but after codec dai's mute_stream(). So the codec driver could do something there before we disable clocks.

thesofproject/linux#2951
#4262

brentlu · 2021-05-28T09:48:29Z

@plbossart @bardliao sorry I was wrong about fsync, it does not come with bclk

Yellow line is the bclk, you can see it start toggling long before the fsync because the SSE bit is set in hw_params

Blue one is the fsync and the purple is the data. Not sure why there is a zero sample before the pcm data we want to send.

Regards,
Brent

bardliao · 2021-05-31T06:44:59Z

@bardliao I can confirm that the nocodec M4 stuff does not work with the MCLK | BCLK case. Using BCLK_ES only gives me the right config with BIT(7) set.

Sorry, I only tested it with 0xc0, but not MCLK | BCLK. I thought they are the same.

That said, when I add you kernel patch to add a 1s delay on startup, I see it but on bclk only. Somehow the FSYNC starts with the data, that's not aligned with what the documentation says.

Can you confirm if you've seen the FSYNC also toggle earlier than the data and start at the same time as BCLK?

No, FSYNC always starts with DATA on my side

CH1: DATA
CH2: BCLK
CH3: FSYNC

plbossart · 2021-06-01T14:52:47Z

Thanks for the comments @bardliao @brentlu. I think we have to double-check the results with our hardware friends.

For the hw_free problem, I am not sure if there is a solution. Even the firmware only deals with the hw_free during the BE dailink hw_free, the clock will always be stopped before the codec hw_free. The codec would need to switch clocks in an earlier step.

brentlu · 2021-06-01T15:40:08Z

@plbossart yes, i finally understand why cs42l42 switches PLL in mute_stream() callback because their hw_free() is always too late regardless platform.

brentlu · 2021-06-06T15:19:56Z

tools/topology/platform/common/ssp.m4

+
 dnl SSP_CONFIG_DATA(type, idx, valid bits, mclk_id, quirks, bclk_delay,
 dnl clks_control, pulse_width, padding)
 dnl mclk_id, quirks, bclk_delay clks_control, pulse_width and padding are optional


it's incomplete...we need to modify the SSP_CONFIG_DATA macro here

not following the feedback @brentlu, what is missing here?
@bardliao I took this code unmodified, not sure what is wrong?

@plbossart I think the issue is that 636cbef is already on the main branch, but @brentlu was testing with glk-012-stable-branch branch where 636cbef is not there.

lyakh · 2021-06-07T08:58:52Z

src/drivers/intel/ssp/ssp.c

there's something weird there: in the #else case below if the condition isn't satisfied we print an error message and jump to out, but we don't set ret... Is that correct?

lyakh · 2021-06-07T09:02:50Z

src/drivers/intel/ssp/ssp.c

if my above suspicion is correct and ret should be negative when jumping to out, then you can just move this assignment above out: and remove the if

lyakh · 2021-06-07T09:04:26Z

src/drivers/intel/ssp/ssp.c

+static int ssp_pre_start(struct dai *dai)
+{
+	struct ssp_pdata *ssp = dai_get_drvdata(dai);
+	int ret = 0;


here the initialisation is indeed redundant

lyakh · 2021-06-07T10:09:33Z

src/drivers/intel/ssp/ssp.c

 	 * ssp_mclk/bclk_prepare_enable/disable functions
 	 */
+	if (!(ssp->params.clks_control &
+		 SOF_DAI_INTEL_SSP_CLKCTRL_MCLK_ES)) {


wrong indentation here and below

lgirdwood · 2021-06-11T10:46:28Z

@bardliao @plbossart should this be closed in favour of #4288 ?

plbossart · 2021-06-11T16:22:16Z

@bardliao @plbossart should this be closed in favour of #4288 ?

yes I think so. #4288 is based on @bardliao 's work so it's not like we have a disagreement.

bardliao requested review from brentlu and keyonjie May 20, 2021 07:02

bardliao force-pushed the ssp-clock branch from 47d8ecd to 5dbb2a0 Compare May 20, 2021 11:26

lgirdwood reviewed May 20, 2021

View reviewed changes

ranj063 reviewed May 20, 2021

View reviewed changes

src/audio/dai.c Outdated

Copy link

Collaborator

ranj063 May 20, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hw_free instead?

brentlu reviewed May 20, 2021

View reviewed changes

src/include/sof/lib/dai.h Outdated Show resolved Hide resolved

bardliao force-pushed the ssp-clock branch from 5dbb2a0 to de509e8 Compare May 21, 2021 02:35

bardliao force-pushed the ssp-clock branch from 9132ed7 to e315cd5 Compare May 21, 2021 11:00

kv2019i added the ABI ABI change involved label May 21, 2021

brentlu reviewed May 23, 2021

View reviewed changes

src/drivers/intel/ssp/ssp.c Outdated

Copy link

Contributor

brentlu May 23, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could bclk continue to hw_free since SSE is clear here?

brentlu reviewed May 23, 2021

View reviewed changes

src/drivers/intel/ssp/ssp.c Outdated Show resolved Hide resolved

brentlu reviewed May 23, 2021

View reviewed changes

bardliao force-pushed the ssp-clock branch from e315cd5 to 806e766 Compare May 24, 2021 02:10

plbossart requested changes May 24, 2021

View reviewed changes

bardliao force-pushed the ssp-clock branch from 806e766 to 44f91ac Compare May 25, 2021 11:16

bardliao removed the ABI ABI change involved label May 25, 2021

bardliao added 2 commits May 25, 2021 20:12

ipc: dai-intel: add SOF_DAI_INTEL_SSP_CLKCTRL_MCLK/BCLK_ES bits

6f2f3db

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. Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>

dai: add dai_hw_free routine

7feb456

So that we can stop ssp clock in dai_reset. Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>

bardliao force-pushed the ssp-clock branch from 44f91ac to 09c656d Compare May 25, 2021 12:13

plbossart requested changes May 25, 2021

View reviewed changes

ranj063 reviewed May 25, 2021

View reviewed changes

bardliao added 2 commits May 27, 2021 09:17

ssp: add hw_params and hw_free dai ops

3ffe417

We will enable/disable ssp clock in the ops. Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>

bardliao force-pushed the ssp-clock branch from c0e47a6 to 8c269e5 Compare May 27, 2021 01:32

brentlu and others added 4 commits May 27, 2021 11:29

topology: ssp: mclk/bclk clock control

0250a67

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>

topology: sof-cnl-nocodec: enable MCLK/BCLK early start

6e9ff5a

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>

topology: sof-apl-nocodec: enable MCLK/BCLK early start

ae5ee43

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>

topology: sof-glk-cs42l42: enable bclk control

ac19390

Enable the bclk clock contrl for SSP2. The 10ms bclk delay is no longer needed since bclk is enabled eariler on hw_params. Signed-off-by: Brent Lu <brent.lu@intel.com> Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>

bardliao force-pushed the ssp-clock branch from 3506108 to ac19390 Compare May 27, 2021 03:54

This was referenced May 28, 2021

ipc: add DAI_HW_FREE IPC command #4262

Closed

ASoC: SOF: add DAI_HW_FREE IPC command thesofproject/linux#2951

Closed

plbossart mentioned this pull request Jun 3, 2021

SSP BLCK and MCLK early start - take3 #4288

Closed

brentlu reviewed Jun 6, 2021

View reviewed changes

lyakh reviewed Jun 7, 2021

View reviewed changes

plbossart closed this Jun 11, 2021

Start ssp clock earlier and stop later #4219

Start ssp clock earlier and stop later #4219

Uh oh!

Conversation

bardliao commented May 20, 2021

Uh oh!

keyonjie commented May 20, 2021

Uh oh!

bardliao commented May 20, 2021

Uh oh!

bardliao commented May 20, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

bardliao commented May 21, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bardliao May 24, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

brentlu May 28, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

brentlu May 23, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

plbossart left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

plbossart May 25, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

bardliao May 24, 2021 •

edited

Loading

brentlu May 28, 2021 •

edited

Loading

brentlu May 23, 2021 •

edited

Loading

plbossart May 25, 2021 •

edited

Loading

ranj063 May 25, 2021 •

edited

Loading