Skip to content

Start ssp clock earlier and stop later #4219

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.

Sign up for GitHub

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

Closed
bardliao wants to merge 9 commits into from
Closed

Start ssp clock earlier and stop later #4219

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
6f2f3db
ipc: dai-intel: add SOF_DAI_INTEL_SSP_CLKCTRL_MCLK/BCLK_ES bits
bardliao May 25, 2021
7feb456
dai: add dai_hw_free routine
bardliao May 20, 2021
93379cd
intel: ssp: introduce ssp_set/release_mclk/bclk helpers to set/releas…
bardliao May 25, 2021
3ffe417
ssp: add hw_params and hw_free dai ops
bardliao May 20, 2021
8c269e5
ssp: handle MCLK/BCLK early start
bardliao May 20, 2021
0250a67
topology: ssp: mclk/bclk clock control
brentlu May 27, 2021
6e9ff5a
topology: sof-cnl-nocodec: enable MCLK/BCLK early start
bardliao May 25, 2021
ae5ee43
topology: sof-apl-nocodec: enable MCLK/BCLK early start
bardliao May 27, 2021
ac19390
topology: sof-glk-cs42l42: enable bclk control
brentlu May 27, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/audio/dai.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -628,6 +628,7 @@ static void dai_config_reset(struct comp_dev *dev)
notifier_unregister(dev, dd->chan,
NOTIFIER_ID_DMA_COPY);
}
dai_hw_free(dd->dai);
}

static int dai_prepare(struct comp_dev *dev)
Expand Down
208 changes: 174 additions & 34 deletions src/drivers/intel/ssp/ssp.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -613,35 +613,49 @@ static int ssp_set_config(struct dai *dai, struct ipc_config_dai *common_config,
return ret;
}

/*
* Portion of the SSP configuration should be applied just before the
* SSP dai is activated, for either power saving or params runtime
* configurable flexibility.
*/
static int ssp_pre_start(struct dai *dai)
static int ssp_mclk_prepare_enable(struct dai *dai)
{
struct ssp_pdata *ssp = dai_get_drvdata(dai);
struct sof_ipc_dai_config *config = &ssp->config;
uint32_t sscr0;
uint32_t mdiv;
bool need_ecs = false;

int ret = 0;

dai_info(dai, "ssp_pre_start()");
int ret;

/* SSP active means bclk already configured. */
if (ssp->state[SOF_IPC_STREAM_PLAYBACK] == COMP_STATE_ACTIVE ||
ssp->state[SOF_IPC_STREAM_CAPTURE] == COMP_STATE_ACTIVE)
if (ssp->clk_active & SSP_CLK_MCLK_ACTIVE)
return 0;

/* MCLK config */
ret = mn_set_mclk(config->ssp.mclk_id, config->ssp.mclk_rate);
if (ret < 0) {
if (ret < 0)
dai_err(dai, "invalid mclk_rate = %d for mclk_id = %d",
config->ssp.mclk_rate, config->ssp.mclk_id);
goto out;
}
else
ssp->clk_active |= SSP_CLK_MCLK_ACTIVE;

return ret;
}

static void ssp_mclk_disable_unprepare(struct dai *dai)
{
struct ssp_pdata *ssp = dai_get_drvdata(dai);

if (!(ssp->clk_active & SSP_CLK_MCLK_ACTIVE))
return;

mn_release_mclk(ssp->config.ssp.mclk_id);

ssp->clk_active &= ~SSP_CLK_MCLK_ACTIVE;
}

static int ssp_bclk_prepare_enable(struct dai *dai)
{
struct ssp_pdata *ssp = dai_get_drvdata(dai);
struct sof_ipc_dai_config *config = &ssp->config;
uint32_t sscr0;
uint32_t mdiv;
bool need_ecs = false;
int ret = 0;
Copy link
Copy Markdown
Collaborator

@ranj063 ranj063 May 25, 2021

Choose a reason for hiding this comment

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

no need to init?

Copy link
Copy Markdown
Collaborator Author

@bardliao bardliao May 26, 2021

Choose a reason for hiding this comment

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

no need to init?

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

Copy link
Copy Markdown
Collaborator

@lyakh lyakh Jun 7, 2021

Choose a reason for hiding this comment

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

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?


if (ssp->clk_active & SSP_CLK_BCLK_ACTIVE)
return 0;

sscr0 = ssp_read(dai, SSCR0);

Expand Down Expand Up @@ -686,6 +700,51 @@ static int ssp_pre_start(struct dai *dai)

dai_info(dai, "ssp_set_config(), sscr0 = 0x%08x", sscr0);
out:
if (!ret)
Copy link
Copy Markdown
Member

@plbossart plbossart May 26, 2021

Choose a reason for hiding this comment

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

nit-pick: for consistency with the rest of the code, use if (ret < 0) ?

Copy link
Copy Markdown
Collaborator Author

@bardliao bardliao May 27, 2021

Choose a reason for hiding this comment

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

The if (ret < 0) in ssp_mclk_prepare_enable is to show some error message, but we already show error message in each error case. We want to set SSP_CLK_BCLK_ACTIVE bit if ret == 0

Copy link
Copy Markdown
Collaborator

@lyakh lyakh Jun 7, 2021

Choose a reason for hiding this comment

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

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

ssp->clk_active |= SSP_CLK_BCLK_ACTIVE;

return ret;
}

static void ssp_bclk_disable_unprepare(struct dai *dai)
{
struct ssp_pdata *ssp = dai_get_drvdata(dai);

if (!(ssp->clk_active & SSP_CLK_BCLK_ACTIVE))
return;
#if CONFIG_INTEL_MN
mn_release_bclk(dai->index);
#endif
ssp->clk_active &= ~SSP_CLK_BCLK_ACTIVE;
}

/*
* Portion of the SSP configuration should be applied just before the
* SSP dai is activated, for either power saving or params runtime
* configurable flexibility.
*/
static int ssp_pre_start(struct dai *dai)
{
struct ssp_pdata *ssp = dai_get_drvdata(dai);
int ret = 0;
Copy link
Copy Markdown
Collaborator

@lyakh lyakh Jun 7, 2021

Choose a reason for hiding this comment

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

here the initialisation is indeed redundant


dai_info(dai, "ssp_pre_start()");

/*
* We will test if mclk/bclk is configured in
* ssp_mclk/bclk_prepare_enable/disable functions
*/
if (!(ssp->params.clks_control &
SOF_DAI_INTEL_SSP_CLKCTRL_MCLK_ES)) {
Copy link
Copy Markdown
Collaborator

@lyakh lyakh Jun 7, 2021

Choose a reason for hiding this comment

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

wrong indentation here and below

/* MCLK config */
ret = ssp_mclk_prepare_enable(dai);
if (ret < 0)
return ret;
}

if (!(ssp->params.clks_control &
SOF_DAI_INTEL_SSP_CLKCTRL_BCLK_ES))
ret = ssp_bclk_prepare_enable(dai);

return ret;
}
Expand All @@ -702,11 +761,16 @@ static void ssp_post_stop(struct dai *dai)
/* release clocks if SSP is inactive */
if (ssp->state[SOF_IPC_STREAM_PLAYBACK] != COMP_STATE_ACTIVE &&
ssp->state[SOF_IPC_STREAM_CAPTURE] != COMP_STATE_ACTIVE) {
dai_info(dai, "releasing BCLK/MCLK clocks for SSP%d...", dai->index);
#if CONFIG_INTEL_MN
mn_release_bclk(dai->index);
#endif
mn_release_mclk(ssp->config.ssp.mclk_id);
if (!(ssp->params.clks_control &
SOF_DAI_INTEL_SSP_CLKCTRL_BCLK_ES)) {
dai_info(dai, "ssp_post_stop releasing BCLK clocks for SSP%d...", dai->index);
ssp_bclk_disable_unprepare(dai);
}
if (!(ssp->params.clks_control &
SOF_DAI_INTEL_SSP_CLKCTRL_MCLK_ES)) {
dai_info(dai, "ssp_post_stop releasing MCLK clocks for SSP%d...", dai->index);
ssp_mclk_disable_unprepare(dai);
}
}
}

Expand Down Expand Up @@ -742,6 +806,78 @@ static int ssp_get_hw_params(struct dai *dai,
return 0;
}

/*
* enabled clock MCLK/BCLK to support codec that need MCLK/BCLK
* This is called before trigger(COMP_TRIGGER_START)
*/
static int ssp_hw_params(struct dai *dai,
struct sof_ipc_stream_params *params)
{
struct ssp_pdata *ssp = dai_get_drvdata(dai);
Copy link
Copy Markdown
Member

@plbossart plbossart May 25, 2021

Choose a reason for hiding this comment

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

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

int ret;

if (ssp->state[SOF_IPC_STREAM_CAPTURE] < COMP_STATE_PREPARE ||
ssp->state[SOF_IPC_STREAM_PLAYBACK] < COMP_STATE_PREPARE) {
dai_err(dai, "ssp_hw_params(): Please set config first");
return -EINVAL;
Comment thread
bardliao marked this conversation as resolved.
Outdated
}

Comment thread
bardliao marked this conversation as resolved.
Outdated
dai_info(dai, "ssp_hw_params() params.clks_control 0x%x", ssp->params.clks_control);
Copy link
Copy Markdown
Contributor

@keyonjie keyonjie May 26, 2021

Choose a reason for hiding this comment

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

We should not set mclk/bclk in case already set, do we need some similar (but not same) logic as below?

	/* SSP active means bclk already configured. */
	if (ssp->state[SOF_IPC_STREAM_PLAYBACK] == COMP_STATE_ACTIVE ||
	    ssp->state[SOF_IPC_STREAM_CAPTURE] == COMP_STATE_ACTIVE)
		return 0;

Copy link
Copy Markdown
Collaborator Author

@bardliao bardliao May 26, 2021

Choose a reason for hiding this comment

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

We should not set mclk/bclk in case already set, do we need some similar (but not same) logic as below?

	/* SSP active means bclk already configured. */
	if (ssp->state[SOF_IPC_STREAM_PLAYBACK] == COMP_STATE_ACTIVE ||
	    ssp->state[SOF_IPC_STREAM_CAPTURE] == COMP_STATE_ACTIVE)
		return 0;

@keyonjie Thanks for pointing it out, but we may not be able to use ssp->state == COMP_STATE_ACTIVE to test if the clock is set. We don't change ssp->state when the clock is set in hw_params. Maybe we need to add a flag to identify the clock status?

Copy link
Copy Markdown
Collaborator Author

@bardliao bardliao May 26, 2021

Choose a reason for hiding this comment

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

@keyonjie updated.


spin_lock(&dai->lock);
Copy link
Copy Markdown
Member

@lgirdwood lgirdwood May 20, 2021

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown
Member

@lgirdwood lgirdwood May 20, 2021

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown
Member

@lgirdwood lgirdwood May 20, 2021

Choose a reason for hiding this comment

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

@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)));

Copy link
Copy Markdown
Contributor

@brentlu brentlu May 23, 2021

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown
Collaborator Author

@bardliao bardliao May 24, 2021

Choose a reason for hiding this comment

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

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?

Copy link
Copy Markdown
Member

@lgirdwood lgirdwood May 24, 2021

Choose a reason for hiding this comment

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

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).


Comment thread
plbossart marked this conversation as resolved.
Outdated
if ((ssp->params.clks_control &
SOF_DAI_INTEL_SSP_CLKCTRL_MCLK_ES)) {
ret = ssp_mclk_prepare_enable(dai);
if (ret < 0)
goto out;
}

if ((ssp->params.clks_control &
SOF_DAI_INTEL_SSP_CLKCTRL_BCLK_ES)) {
ret = ssp_bclk_prepare_enable(dai);
if (ret < 0)
goto out;
/* enable port */
ssp_update_bits(dai, SSCR0, SSCR0_SSE, SSCR0_SSE);
}
out:

spin_unlock(&dai->lock);
Copy link
Copy Markdown
Collaborator

@ranj063 ranj063 May 25, 2021

Choose a reason for hiding this comment

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

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?

Copy link
Copy Markdown
Collaborator Author

@bardliao bardliao May 26, 2021

Choose a reason for hiding this comment

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

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.

return 0;
}

/*
* disabled clock MCLK/BCLK to support codec that need MCLK/BCLK
* This is called after trigger(COMP_TRIGGER_STOP)
*/
static void ssp_hw_free(struct dai *dai)
{
struct ssp_pdata *ssp = dai_get_drvdata(dai);

dai_info(dai, "ssp_hw_free()");

spin_lock(&dai->lock);

/* release clocks if SSP is inactive */
if (ssp->state[SOF_IPC_STREAM_CAPTURE] == COMP_STATE_PREPARE &&
ssp->state[SOF_IPC_STREAM_PLAYBACK] == COMP_STATE_PREPARE) {
if ((ssp->params.clks_control &
SOF_DAI_INTEL_SSP_CLKCTRL_BCLK_ES)) {
dai_info(dai, "ssp_hw_free() releasing BCLK clocks for SSP%d...", dai->index);
ssp_update_bits(dai, SSCR0, SSCR0_SSE, 0);
ssp_bclk_disable_unprepare(dai);
}
if ((ssp->params.clks_control &
SOF_DAI_INTEL_SSP_CLKCTRL_MCLK_ES)) {
dai_info(dai, "ssp_hw_free releasing MCLK clocks for SSP%d...", dai->index);
ssp_mclk_disable_unprepare(dai);
}
}

spin_unlock(&dai->lock);
}

/* start the SSP for either playback or capture */
static void ssp_start(struct dai *dai, int direction)
{
Expand All @@ -752,8 +888,11 @@ static void ssp_start(struct dai *dai, int direction)
/* request mclk/bclk */
ssp_pre_start(dai);

/* enable port */
ssp_update_bits(dai, SSCR0, SSCR0_SSE, SSCR0_SSE);
if (!(ssp->params.clks_control &
SOF_DAI_INTEL_SSP_CLKCTRL_BCLK_ES)) {
/* enable port */
ssp_update_bits(dai, SSCR0, SSCR0_SSE, SSCR0_SSE);
Copy link
Copy Markdown
Member

@plbossart plbossart May 24, 2021

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown
Collaborator

@ranj063 ranj063 May 25, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

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

}
Copy link
Copy Markdown
Member

@plbossart plbossart May 24, 2021

Choose a reason for hiding this comment

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

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

ssp->state[direction] = COMP_STATE_ACTIVE;

dai_info(dai, "ssp_start()");
Expand Down Expand Up @@ -814,8 +953,11 @@ static void ssp_stop(struct dai *dai, int direction)
/* disable SSP port if no users */
if (ssp->state[SOF_IPC_STREAM_CAPTURE] == COMP_STATE_PREPARE &&
ssp->state[SOF_IPC_STREAM_PLAYBACK] == COMP_STATE_PREPARE) {
ssp_update_bits(dai, SSCR0, SSCR0_SSE, 0);
dai_info(dai, "ssp_stop(), SSP port disabled");
if (!(ssp->params.clks_control &
SOF_DAI_INTEL_SSP_CLKCTRL_BCLK_ES)) {
ssp_update_bits(dai, SSCR0, SSCR0_SSE, 0);
dai_info(dai, "ssp_stop(), SSP port disabled");
}
}

ssp_post_stop(dai);
Expand Down Expand Up @@ -907,14 +1049,10 @@ static int ssp_probe(struct dai *dai)

static int ssp_remove(struct dai *dai)
{
struct ssp_pdata *ssp = dai_get_drvdata(dai);

pm_runtime_put_sync(SSP_CLK, dai->index);

mn_release_mclk(ssp->config.ssp.mclk_id);
#if CONFIG_INTEL_MN
mn_release_bclk(dai->index);
#endif
ssp_mclk_disable_unprepare(dai);
ssp_bclk_disable_unprepare(dai);

/* Disable SSP power */
pm_runtime_put_sync(SSP_POW, dai->index);
Expand Down Expand Up @@ -949,6 +1087,8 @@ const struct dai_driver ssp_driver = {
.get_hw_params = ssp_get_hw_params,
.get_handshake = ssp_get_handshake,
.get_fifo = ssp_get_fifo,
.hw_params = ssp_hw_params,
.hw_free = ssp_hw_free,
.probe = ssp_probe,
.remove = ssp_remove,
},
Expand Down
4 changes: 4 additions & 0 deletions src/include/ipc/dai-intel.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,10 @@
#define SOF_DAI_INTEL_SSP_CLKCTRL_FS_KA BIT(4)
/* bclk idle */
#define SOF_DAI_INTEL_SSP_CLKCTRL_BCLK_IDLE_HIGH BIT(5)
/* mclk early start */
#define SOF_DAI_INTEL_SSP_CLKCTRL_MCLK_ES BIT(6)
/* bclk early start */
#define SOF_DAI_INTEL_SSP_CLKCTRL_BCLK_ES BIT(7)

/* DMIC max. four controllers for eight microphone channels */
#define SOF_DAI_INTEL_DMIC_NUM_CTRL 4
Expand Down
4 changes: 4 additions & 0 deletions src/include/sof/drivers/ssp.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,12 +224,16 @@ extern const struct dai_driver ssp_driver;
#define ssp_irq(ssp) \
ssp->plat_data.irq

#define SSP_CLK_MCLK_ACTIVE BIT(0)
#define SSP_CLK_BCLK_ACTIVE BIT(1)

/* SSP private data */
struct ssp_pdata {
uint32_t sscr0;
uint32_t sscr1;
uint32_t psp;
uint32_t state[2]; /* SSP_STATE_ for each direction */
uint32_t clk_active;
struct sof_ipc_dai_config config;
struct sof_ipc_dai_ssp_params params;
};
Expand Down
7 changes: 7 additions & 0 deletions src/include/sof/lib/dai.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ struct dai_ops {
int (*get_hw_params)(struct dai *dai,
struct sof_ipc_stream_params *params, int dir);
int (*hw_params)(struct dai *dai, struct sof_ipc_stream_params *params);
void (*hw_free)(struct dai *dai);
int (*get_handshake)(struct dai *dai, int direction, int stream_id);
Comment thread
bardliao marked this conversation as resolved.
Outdated
int (*get_fifo)(struct dai *dai, int direction, int stream_id);
int (*probe)(struct dai *dai);
Expand Down Expand Up @@ -423,6 +424,12 @@ static inline int dai_hw_params(struct dai *dai,
return ret;
}

static inline void dai_hw_free(struct dai *dai)
{
if (dai->drv->ops.hw_free)
dai->drv->ops.hw_free(dai);
}

/**
* \brief Get Digital Audio interface DMA Handshake
*/
Expand Down
5 changes: 5 additions & 0 deletions tools/topology/platform/common/ssp.m4
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,11 @@ $6
dnl SSP_QUIRK_LBM 64 = (1 << 6)
define(`SSP_QUIRK_LBM', 64)

dnl SSP_CC_MCLK_ES 64 = (1 << 6)
define(`SSP_CC_MCLK_ES', 64)
dnl SSP_CC_BCLK_ES 128 = (1 << 7)
define(`SSP_CC_BCLK_ES', 128)

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
Copy link
Copy Markdown
Contributor

@brentlu brentlu Jun 6, 2021

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown
Member

@plbossart plbossart Jun 7, 2021

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown
Collaborator Author

@bardliao bardliao Jun 8, 2021

Choose a reason for hiding this comment

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

@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.

Expand Down
18 changes: 12 additions & 6 deletions tools/topology/sof-apl-nocodec.m4
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -272,42 +272,48 @@ DAI_CONFIG(SSP, 0, 0, NoCodec-0,
SSP_CLOCK(fsync, 48000, codec_slave),
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)))
SSP_CONFIG_DATA(SSP, 0, 16, 0, SSP_QUIRK_LBM, 0,
SSP_CC_MCLK_ES | SSP_CC_BCLK_ES)))

DAI_CONFIG(SSP, 1, 1, NoCodec-1,
SSP_CONFIG(I2S, SSP_CLOCK(mclk, 24576000, codec_mclk_in),
SSP_CLOCK(bclk, 1536000, codec_slave),
SSP_CLOCK(fsync, 48000, codec_slave),
SSP_TDM(2, 16, 3, 3),
SSP_CONFIG_DATA(SSP, 1, 16, 0, SSP_QUIRK_LBM)))
SSP_CONFIG_DATA(SSP, 1, 16, 0, SSP_QUIRK_LBM, 0,
SSP_CC_MCLK_ES | SSP_CC_BCLK_ES)))

#DAI_CONFIG(SSP, 2, 2, NoCodec-2,
# SSP_CONFIG(I2S, SSP_CLOCK(mclk, 24576000, codec_mclk_in),
# SSP_CLOCK(bclk, 1536000, codec_slave),
# SSP_CLOCK(fsync, 48000, codec_slave),
# SSP_TDM(2, 16, 3, 3),
# SSP_CONFIG_DATA(SSP, 2, 16, 0, SSP_QUIRK_LBM)))
# SSP_CONFIG_DATA(SSP, 2, 16, 0, SSP_QUIRK_LBM, 0,
# SSP_CC_MCLK_ES | SSP_CC_BCLK_ES)))

DAI_CONFIG(SSP, 3, 3, NoCodec-3,
SSP_CONFIG(I2S, SSP_CLOCK(mclk, 24576000, codec_mclk_in),
SSP_CLOCK(bclk, 1536000, codec_slave),
SSP_CLOCK(fsync, 48000, codec_slave),
SSP_TDM(2, 16, 3, 3),
SSP_CONFIG_DATA(SSP, 3, 16, 0, SSP_QUIRK_LBM)))
SSP_CONFIG_DATA(SSP, 3, 16, 0, SSP_QUIRK_LBM, 0,
SSP_CC_MCLK_ES | SSP_CC_BCLK_ES)))

#DAI_CONFIG(SSP, 4, 4, NoCodec-4,
# SSP_CONFIG(I2S, SSP_CLOCK(mclk, 24576000, codec_mclk_in),
# SSP_CLOCK(bclk, 1536000, codec_slave),
# SSP_CLOCK(fsync, 48000, codec_slave),
# SSP_TDM(2, 16, 3, 3),
# SSP_CONFIG_DATA(SSP, 4, 16, 0, SSP_QUIRK_LBM)))
# SSP_CONFIG_DATA(SSP, 4, 16, 0, SSP_QUIRK_LBM, 0,
# SSP_CC_MCLK_ES | SSP_CC_BCLK_ES)))

DAI_CONFIG(SSP, 5, 5, NoCodec-5,
SSP_CONFIG(I2S, SSP_CLOCK(mclk, 24576000, codec_mclk_in),
SSP_CLOCK(bclk, 1536000, codec_slave),
SSP_CLOCK(fsync, 48000, codec_slave),
SSP_TDM(2, 16, 3, 3),
SSP_CONFIG_DATA(SSP, 5, 16, 0, SSP_QUIRK_LBM)))
SSP_CONFIG_DATA(SSP, 5, 16, 0, SSP_QUIRK_LBM, 0,
SSP_CC_MCLK_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,
Expand Down
Loading