Re: [PATCH v2] ASoC: rt5670: add set_bclk_ratio in dai ops

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 9/13/17 10:14 PM, Bard Liao wrote:
-----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.

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/cht_bsw_rt5672.c#L211






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




[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux