On 9/13/17 10:14 PM, Bard Liao wrote:
-----Original Message----- From: Pierre-Louis Bossart [mailto:pierre-louis.bossart@linux.intel.com] Sent: Thursday, September 14, 2017 1:45 AM To: Bard Liao; broonie@kernel.org; lgirdwood@gmail.com Cc: Oder Chiou; Jack Yu; alsa-devel@alsa-project.org; lars@metafoo.de; tiwai@suse.de; Shuming [范書銘]; Flove(HsinFu) Subject: Re: [alsa-devel] [PATCH v2] ASoC: rt5670: add set_bclk_ratio in dai ops
On 9/12/17 11:35 PM, Bard Liao wrote:
-----Original Message----- From: Pierre-Louis Bossart [mailto:pierre-louis.bossart@linux.intel.com] Sent: Tuesday, September 12, 2017 10:54 PM To: Bard Liao; broonie@kernel.org; lgirdwood@gmail.com Cc: Oder Chiou; Jack Yu; alsa-devel@alsa-project.org; lars@metafoo.de; tiwai@suse.de; Shuming [范書銘]; Flove(HsinFu) Subject: Re: [alsa-devel] [PATCH v2] ASoC: rt5670: add set_bclk_ratio in dai
ops
On 9/12/17 3:12 AM, Bard Liao wrote:
We need to set a specific bit for 50 bclk rate. So add set_bclk_ratio function to set the bit.
When is this supposed to be used? the cht_bsw_rt5672 machine driver uses a 19.2MHz MCLK/2.4 MHz bclk, so there's a typical 50x ratio.
I think it should be used for all Intel platform. The bit will only be effective in PCM mode with TDM enabled. It will influence the data allocation format. 0: the dummy bits will be allocate at the end of each slot. 1: the dummy bits will be allocate at the end of all slots. For example, 50fs with 24 bits data length. 0: 24 bits slot0 data + 1 dummy bit + 24 bits slot1 data + 1 dummy bit 1: 24 bits slot0 data + 24 bits slot1 data + 2 dummy bits
I will do slight modification on the patch since the bit should be set for all bclk rate which is divisible by 50.
I am not sure I understand the 0: and 1: cases, and since we get audio today with this codec and a ratio of 200 (4 24-bit slots in DSP_B) I don't get what happens with this fix.
From my experience, most Intel platforms use case 1 format. The issue will be incorrect recording volume due to bit allocation is shifted. If Intel expected case 1 format but Realtek send case 0 format. It would be no problem with slot 0. But for slot 1, it will be: Intel expected: 24 bits valid data for slot 1 Realtek sent: 1 dummy bit for slot 0 + 24 bits data for slot 1 So what Intel actually received will be 1 dummy bit + 23 bits data And it is similar to other slots. BTW, if you don't meet the issue, you don't need to call snd_soc_dai_set_bclk_ratio on your machine driver.
Yes, the Intel SSP will push out all slots and dummy bits at the end of the frame. But if this was never enabled we should have seen noise on the right channel on playback - this sort of misalignment is pretty obvious to spot. I also see a mention of limitation to 50fs resulting in ASRC being required.
http://elixir.free-electrons.com/linux/latest/source/sound/soc/intel/boards/...
Signed-off-by: Bard Liao bardliao@realtek.com
v2:
- add define for tdm data format select bit.
sound/soc/codecs/rt5670.c | 19 +++++++++++++++++++ sound/soc/codecs/rt5670.h | 4 ++++ 2 files changed, 23 insertions(+)diff --git a/sound/soc/codecs/rt5670.c b/sound/soc/codecs/rt5670.c index 9545764..da7614c 100644 --- a/sound/soc/codecs/rt5670.c +++ b/sound/soc/codecs/rt5670.c @@ -2582,6 +2582,24 @@ static int rt5670_set_tdm_slot(struct
snd_soc_dai
*dai, unsigned int tx_mask,
return 0; }+static int rt5670_set_bclk_ratio(struct snd_soc_dai *dai, unsigned int
ratio)
+{
- struct snd_soc_codec *codec = dai->codec;
- dev_dbg(codec->dev, "%s ratio=%d\n", __func__, ratio);
- if (dai->id != RT5670_AIF1)
return 0;- if (ratio == 50)
snd_soc_update_bits(codec, RT5670_GEN_CTRL3,RT5670_TDM_DATA_MODE_SEL,RT5670_TDM_DATA_MODE_50FS);
- else
snd_soc_update_bits(codec, RT5670_GEN_CTRL3,RT5670_TDM_DATA_MODE_SEL,RT5670_TDM_DATA_MODE_NOR);
- return 0;
+}
- static int rt5670_set_bias_level(struct snd_soc_codec *codec, enum snd_soc_bias_level level) {
@@ -2712,6 +2730,7 @@ static const struct snd_soc_dai_ops
rt5670_aif_dai_ops = {
.set_fmt = rt5670_set_dai_fmt, .set_tdm_slot = rt5670_set_tdm_slot, .set_pll = rt5670_set_dai_pll,
.set_bclk_ratio = rt5670_set_bclk_ratio, };
static struct snd_soc_dai_driver rt5670_dai[] = {
diff --git a/sound/soc/codecs/rt5670.h b/sound/soc/codecs/rt5670.h index 5ba485c..265df80 100644 --- a/sound/soc/codecs/rt5670.h +++ b/sound/soc/codecs/rt5670.h @@ -1816,6 +1816,10 @@ #define RT5670_ZCD_HP_DIS (0x0 << 15) #define RT5670_ZCD_HP_EN (0x1 << 15)
+/* General Control 3 (0xfc) */ +#define RT5670_TDM_DATA_MODE_SEL (0x1 << 11) +#define RT5670_TDM_DATA_MODE_NOR (0x0 << 11) +#define RT5670_TDM_DATA_MODE_50FS (0x1 << 11)
/* Codec Private Register definition */ /* 3D Speaker Control (0x63) */------Please consider the environment before printing this e-mail.
Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel