> -----Original Message----- > From: Pierre-Louis Bossart [mailto:pierre-louis.bossart@xxxxxxxxxxxxxxx] > Sent: Thursday, September 14, 2017 1:45 AM > To: Bard Liao; broonie@xxxxxxxxxx; lgirdwood@xxxxxxxxx > Cc: Oder Chiou; Jack Yu; alsa-devel@xxxxxxxxxxxxxxxx; lars@xxxxxxxxxx; > tiwai@xxxxxxx; Shuming [范書銘]; Flove(HsinFu) > Subject: Re: [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@xxxxxxxxxxxxxxx] > >> Sent: Tuesday, September 12, 2017 10:54 PM > >> To: Bard Liao; broonie@xxxxxxxxxx; lgirdwood@xxxxxxxxx > >> Cc: Oder Chiou; Jack Yu; alsa-devel@xxxxxxxxxxxxxxxx; lars@xxxxxxxxxx; > >> tiwai@xxxxxxx; Shuming [范書銘]; Flove(HsinFu) > >> Subject: Re: [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. > > > > >> > >>> > >>> Signed-off-by: Bard Liao <bardliao@xxxxxxxxxxx> > >>> --- > >>> 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@xxxxxxxxxxxxxxxx > > http://mailman.alsa-project.org/mailman/listinfo/alsa-devel > > _______________________________________________ Alsa-devel mailing list Alsa-devel@xxxxxxxxxxxxxxxx http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
