From 53982acd9f448f365da337b0e2f3eb2db7428f73 Mon Sep 17 00:00:00 2001 From: bunnie Date: Fri, 7 May 2021 14:17:49 +0800 Subject: [PATCH] I2S fix: sample SYNC on the correct edge (#904) * resolve issue #862 add description to soc.svd The issue is that with no description provided it simply would not put out a description tag, which breaks compatibility with other programs. Insert a somewhat useful default description including a timestamp and the words "LiteX SoC". * I2S fix: sample SYNC on the correct edge The original Tx path implementation samples SYNC on the falling edge, out of convenience with the fact that teh data must also change on the falling edge. This works OK, until you have a CODEC which has a ~40ns max delay spec on the SYNC, and also has a slightly asymmetric SYNC edge (the SYNC signal is also the WCLK or LRCLK depending on which docs you read). The SYNC by spec is supposed to change on the falling edge, and this extra delay is enough to cause the SYNC to introduce occassional bit or frame shifts into the audio. This fix samples the SYNC on the rising edge, but still changes the data on the falling edge, thus allowing for implementations where SYNC has quite loose timings relative to everything else (as is the case on the TLV320AIC3200) --- litex/soc/cores/i2s.py | 35 +++++++++++++++++++++++------------ 1 file changed, 23 insertions(+), 12 deletions(-) diff --git a/litex/soc/cores/i2s.py b/litex/soc/cores/i2s.py index 5f51a1a95..9631dcc4b 100644 --- a/litex/soc/cores/i2s.py +++ b/litex/soc/cores/i2s.py @@ -559,19 +559,25 @@ class S7I2S(Module, AutoCSR, AutoDoc): self.submodules.txi2s = txi2s = FSM(reset_state="IDLE") txi2s.act("IDLE", If(self.tx_ctl.fields.enable, - If(falling_edge & (~sync_pin if frame_format == I2S_FORMAT.I2S_STANDARD else sync_pin), + If(rising_edge & (~sync_pin if frame_format == I2S_FORMAT.I2S_STANDARD else sync_pin), NextState("WAIT_SYNC"), ) ) ), txi2s.act("WAIT_SYNC", - If(falling_edge & (~sync_pin if frame_format == I2S_FORMAT.I2S_STANDARD else sync_pin), - NextState("LEFT"), + If(rising_edge & (~sync_pin if frame_format == I2S_FORMAT.I2S_STANDARD else sync_pin), + NextState("LEFT_FALL"), NextValue(tx_cnt, sample_width), NextValue(tx_buf, Cat(tx_rd_d, offset)), tx_rden.eq(1), ) ) + # sync should be sampled on rising edge, but data should change on falling edge + txi2s.act("LEFT_FALL", + If(falling_edge, + NextState("LEFT") + ) + ) txi2s.act("LEFT", If(~self.tx_ctl.fields.enable, NextState("IDLE") @@ -587,7 +593,7 @@ class S7I2S(Module, AutoCSR, AutoDoc): If(~self.tx_ctl.fields.enable, NextState("IDLE") ).Else( - If(falling_edge, + If(rising_edge, If((tx_cnt == 0), If((sync_pin if frame_format == I2S_FORMAT.I2S_STANDARD else ~sync_pin), NextValue(tx_cnt, sample_width), @@ -596,7 +602,7 @@ class S7I2S(Module, AutoCSR, AutoDoc): NextState("LEFT_WAIT"), ) ).Elif(tx_cnt > 0, - NextState("LEFT"), + NextState("LEFT_FALL"), ) ) ) @@ -606,23 +612,28 @@ class S7I2S(Module, AutoCSR, AutoDoc): If(~self.tx_ctl.fields.enable, NextState("IDLE") ).Else( - If(falling_edge, + If(rising_edge, If((tx_cnt == 0), If((sync_pin if frame_format == I2S_FORMAT.I2S_STANDARD else ~sync_pin), NextValue(tx_cnt, sample_width), - NextState("RIGHT"), + NextState("RIGHT_FALL"), NextValue(tx_buf, Cat(tx_rd_d,offset)), tx_rden.eq(1), ).Else( NextState("LEFT_WAIT"), ) ).Elif(tx_cnt > 0, - NextState("LEFT"), + NextState("LEFT_FALL"), ) ) ) ) - + # sync should be sampled on rising edge, but data should change on falling edge + txi2s.act("RIGHT_FALL", + If(falling_edge, + NextState("RIGHT") + ) + ) txi2s.act("RIGHT", If(~self.tx_ctl.fields.enable, NextState("IDLE") @@ -637,14 +648,14 @@ class S7I2S(Module, AutoCSR, AutoDoc): If(~self.tx_ctl.fields.enable, NextState("IDLE") ).Else( - If(falling_edge, + If(rising_edge, If((tx_cnt == 0) & (~sync_pin if frame_format == I2S_FORMAT.I2S_STANDARD else sync_pin), NextValue(tx_cnt, sample_width), - NextState("LEFT"), + NextState("LEFT_FALL"), NextValue(tx_buf, Cat(tx_rd_d,offset)), tx_rden.eq(1) ).Elif(tx_cnt > 0, - NextState("RIGHT") + NextState("RIGHT_FALL") ) ) )