[Sound-open-firmware] [RFC PATCH 0/6] Cleanups for SSP slave support on APL/CNL
While looking at the settings needed for the SSP slave support, I came across severial confusions or errors. I still can't get this mode to work but I could use feedback on the following patches, and also double-check if there is any impact in master mode.
Comments/test welcome.
Pierre-Louis Bossart (6): apl-ssp: fix SSIOC_SCOE apl-ssp: fix SSIOC_SFCR apl-ssp: fix SCR1_SCFR apl-ssp: move SSCR0_ACS,MOD as default settings apl-ssp: fix SSCR0_ECS apl-ssp: fix SSCR1_SFRMDIR
src/drivers/apl-ssp.c | 26 +++++++++----------------- 1 file changed, 9 insertions(+), 17 deletions(-)
This register enables the bclk output and only makes sense in codec slave mode (SoC drives the bit clock)
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- src/drivers/apl-ssp.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/src/drivers/apl-ssp.c b/src/drivers/apl-ssp.c index fd5905c..af0ad63 100644 --- a/src/drivers/apl-ssp.c +++ b/src/drivers/apl-ssp.c @@ -190,8 +190,8 @@ static inline int ssp_set_config(struct dai *dai, /* sspsp2 no dynamic setting */ sspsp2 = 0x0;
- /* ssioc dynamic setting is SFCR */ - ssioc = SSIOC_SCOE; + /* ssioc dynamic setting is SCOE, SFCR */ + ssioc = 0x0;
/* i2s_m M divider setting, default 1 */ i2s_m = 0x1; @@ -224,6 +224,7 @@ static inline int ssp_set_config(struct dai *dai, case SOF_DAI_FMT_CBS_CFS: sscr1 |= SSCR1_SCFR; ssioc |= SSIOC_SFCR; + ssioc |= SSIOC_SCOE; break; case SOF_DAI_FMT_CBM_CFS: sscr0 |= SSCR0_ECS; /* external clock used */ @@ -235,6 +236,7 @@ static inline int ssp_set_config(struct dai *dai, /* FIXME: this mode has not been tested */ break; case SOF_DAI_FMT_CBS_CFM: + ssioc |= SSIOC_SCOE; sscr1 |= SSCR1_SCFR; /* FIXME: this mode has not been tested */ break;
Not sure what we'd need the bit clock to keep running.
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- src/drivers/apl-ssp.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/src/drivers/apl-ssp.c b/src/drivers/apl-ssp.c index af0ad63..c0b4661 100644 --- a/src/drivers/apl-ssp.c +++ b/src/drivers/apl-ssp.c @@ -190,7 +190,7 @@ static inline int ssp_set_config(struct dai *dai, /* sspsp2 no dynamic setting */ sspsp2 = 0x0;
- /* ssioc dynamic setting is SCOE, SFCR */ + /* ssioc dynamic setting is SCOE */ ssioc = 0x0;
/* i2s_m M divider setting, default 1 */ @@ -223,7 +223,6 @@ static inline int ssp_set_config(struct dai *dai, break; case SOF_DAI_FMT_CBS_CFS: sscr1 |= SSCR1_SCFR; - ssioc |= SSIOC_SFCR; ssioc |= SSIOC_SCOE; break; case SOF_DAI_FMT_CBM_CFS:
This bitfield was used in codec slave mode when it's intended for the codec master mode - though it's not clear if it's useful to signal that the external clock is continuously running.
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- src/drivers/apl-ssp.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/src/drivers/apl-ssp.c b/src/drivers/apl-ssp.c index c0b4661..b706a20 100644 --- a/src/drivers/apl-ssp.c +++ b/src/drivers/apl-ssp.c @@ -172,7 +172,7 @@ static inline int ssp_set_config(struct dai *dai, */ sscr0 = SSCR0_PSP | SSCR0_RIM | SSCR0_TIM;
- /* sscr1 dynamic settings are SFRMDIR, SCLKDIR, SCFR */ + /* sscr1 dynamic settings are SFRMDIR, SCLKDIR */ sscr1 = SSCR1_TTE | SSCR1_TTELP | SSCR1_TRAIL | SSCR1_RSRE | SSCR1_TSRE;
/* sscr2 dynamic setting is LJDFD */ @@ -222,7 +222,6 @@ static inline int ssp_set_config(struct dai *dai, */ break; case SOF_DAI_FMT_CBS_CFS: - sscr1 |= SSCR1_SCFR; ssioc |= SSIOC_SCOE; break; case SOF_DAI_FMT_CBM_CFS: @@ -236,7 +235,6 @@ static inline int ssp_set_config(struct dai *dai, break; case SOF_DAI_FMT_CBS_CFM: ssioc |= SSIOC_SCOE; - sscr1 |= SSCR1_SCFR; /* FIXME: this mode has not been tested */ break; default:
These settings are not conditionally set, move as init values
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- src/drivers/apl-ssp.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/src/drivers/apl-ssp.c b/src/drivers/apl-ssp.c index b706a20..e1e2ca1 100644 --- a/src/drivers/apl-ssp.c +++ b/src/drivers/apl-ssp.c @@ -165,12 +165,10 @@ static inline int ssp_set_config(struct dai *dai, trace_value(config->format);
/* reset SSP settings */ + /* sscr0 dynamic settings are DSS, EDSS, SCR, FRDC, ECS */ - /* - * FIXME: MOD, ACS, NCS are not set, - * no support for network mode for now - */ sscr0 = SSCR0_PSP | SSCR0_RIM | SSCR0_TIM; + sscr0 |= SSCR0_MOD | SSCR0_ACS;
/* sscr1 dynamic settings are SFRMDIR, SCLKDIR */ sscr1 = SSCR1_TTE | SSCR1_TTELP | SSCR1_TRAIL | SSCR1_RSRE | SSCR1_TSRE; @@ -263,8 +261,6 @@ static inline int ssp_set_config(struct dai *dai, goto out; }
- sscr0 |= SSCR0_MOD | SSCR0_ACS; - mdivc = 0x1; #ifdef CONFIG_CANNONLAKE if (!config->ssp.mclk_rate || config->ssp.mclk_rate > F_24000_kHz) {
This bitfield controls the use of the external oscillator or PLL, it has nothing to do with the bclk master settings.
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- src/drivers/apl-ssp.c | 2 -- 1 file changed, 2 deletions(-)
diff --git a/src/drivers/apl-ssp.c b/src/drivers/apl-ssp.c index e1e2ca1..fdd40bd 100644 --- a/src/drivers/apl-ssp.c +++ b/src/drivers/apl-ssp.c @@ -212,7 +212,6 @@ static inline int ssp_set_config(struct dai *dai, trace_value(config->format); switch (config->format & SOF_DAI_FMT_MASTER_MASK) { case SOF_DAI_FMT_CBM_CFM: - sscr0 |= SSCR0_ECS; /* external clock used */ sscr1 |= SSCR1_SCLKDIR; /* * FIXME: does SSRC1.SCFR need to be set @@ -223,7 +222,6 @@ static inline int ssp_set_config(struct dai *dai, ssioc |= SSIOC_SCOE; break; case SOF_DAI_FMT_CBM_CFS: - sscr0 |= SSCR0_ECS; /* external clock used */ sscr1 |= SSCR1_SCLKDIR; /* * FIXME: does SSRC1.SCFR need to be set
This bitfield indicates the codec is frame master (CFM), set it as needed. The existing code could only deal with the SoC frame master (CFS)
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- src/drivers/apl-ssp.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/src/drivers/apl-ssp.c b/src/drivers/apl-ssp.c index fdd40bd..c9f8a7c 100644 --- a/src/drivers/apl-ssp.c +++ b/src/drivers/apl-ssp.c @@ -206,13 +206,11 @@ static inline int ssp_set_config(struct dai *dai, /* ssrsa dynamic setting is RTSA, default 2 slots */ ssrsa = config->ssp.rx_slots;
- /* clock masters */ - sscr1 &= ~SSCR1_SFRMDIR; - trace_value(config->format); switch (config->format & SOF_DAI_FMT_MASTER_MASK) { case SOF_DAI_FMT_CBM_CFM: sscr1 |= SSCR1_SCLKDIR; + sscr1 |= SSCR1_SFRMDIR; /* * FIXME: does SSRC1.SCFR need to be set * when codec is master ? @@ -231,6 +229,7 @@ static inline int ssp_set_config(struct dai *dai, break; case SOF_DAI_FMT_CBS_CFM: ssioc |= SSIOC_SCOE; + sscr1 |= SSCR1_SFRMDIR; /* FIXME: this mode has not been tested */ break; default:
participants (1)
-
Pierre-Louis Bossart