| Message ID | 20200903203034.1057334-1-peron.clem@gmail.com |
|---|---|
| Headers | show |
| Series | Add Allwinner H3/H5/H6/A64 HDMI audio | expand |
Hi On Thu, Sep 03, 2020 at 10:30:15PM +0200, Clément Péron wrote: > This reverts commit dd657eae8164f7e4bafe8b875031a7c6c50646a9. > > There was a misinterpretation of the analysis using a scope. > After rechecking this using a logical analyzer the LRCK polarity is > fine. Yes, it's fine indeed, so there's no need to change anything? Maxime
Hi Samuel, On Fri, 4 Sep 2020 at 05:16, Samuel Holland <samuel@sholland.org> wrote: > > Clément, > > On 9/3/20 3:30 PM, Clément Péron wrote: > > From: Jernej Skrabec <jernej.skrabec@siol.net> > > > > H6 I2S is very similar to that in H3, except it supports up to 16 > > channels. > > > > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net> > > Signed-off-by: Marcus Cooper <codekipper@gmail.com> > > Signed-off-by: Clément Péron <peron.clem@gmail.com> > > --- > > sound/soc/sunxi/sun4i-i2s.c | 221 ++++++++++++++++++++++++++++++++++++ > > 1 file changed, 221 insertions(+) > > > > diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c > > index fabff7bcccbc..acf24f512f2c 100644 > > --- a/sound/soc/sunxi/sun4i-i2s.c > > +++ b/sound/soc/sunxi/sun4i-i2s.c > > [snip] > > > @@ -474,6 +489,65 @@ static int sun8i_i2s_set_chan_cfg(const struct sun4i_i2s *i2s, > > return 0; > > } > > > > +static int sun50i_h6_i2s_set_chan_cfg(const struct sun4i_i2s *i2s, > > + const struct snd_pcm_hw_params *params) > > +{ > > + unsigned int channels = params_channels(params); > > + unsigned int slots = channels; > > + unsigned int lrck_period; > > + > > + if (i2s->slots) > > + slots = i2s->slots; > > + > > + /* Map the channels for playback and capture */ > > + regmap_write(i2s->regmap, SUN50I_H6_I2S_TX_CHAN_MAP1_REG, 0x76543210); > > + regmap_write(i2s->regmap, SUN50I_H6_I2S_RX_CHAN_MAP1_REG, 0x76543210); > > + > > + /* Configure the channels */ > > + regmap_update_bits(i2s->regmap, SUN8I_I2S_TX_CHAN_SEL_REG, > > + SUN50I_H6_I2S_TX_CHAN_SEL_MASK, > > + SUN50I_H6_I2S_TX_CHAN_SEL(channels)); > > + regmap_update_bits(i2s->regmap, SUN50I_H6_I2S_RX_CHAN_SEL_REG, > > + SUN50I_H6_I2S_TX_CHAN_SEL_MASK, > > + SUN50I_H6_I2S_TX_CHAN_SEL(channels)); > > + > > + regmap_update_bits(i2s->regmap, SUN8I_I2S_CHAN_CFG_REG, > > + SUN8I_I2S_CHAN_CFG_TX_SLOT_NUM_MASK, > > + SUN8I_I2S_CHAN_CFG_TX_SLOT_NUM(channels)); > > + regmap_update_bits(i2s->regmap, SUN8I_I2S_CHAN_CFG_REG, > > + SUN8I_I2S_CHAN_CFG_RX_SLOT_NUM_MASK, > > + SUN8I_I2S_CHAN_CFG_RX_SLOT_NUM(channels)); > > + > > + switch (i2s->format & SND_SOC_DAIFMT_FORMAT_MASK) { > > + case SND_SOC_DAIFMT_DSP_A: > > + case SND_SOC_DAIFMT_DSP_B: > > + case SND_SOC_DAIFMT_LEFT_J: > > + case SND_SOC_DAIFMT_RIGHT_J: > > These cases don't match the documentation: LEFT_J and RIGHT_J are documented to > behave like I2S (lrck_period == slot_width), not like DSP_A/B (lrck_period == > slot_width * slots). > > > + lrck_period = params_physical_width(params) * slots; > > + break; > > + > > + case SND_SOC_DAIFMT_I2S: > > + lrck_period = params_physical_width(params); > > + break; > > + > > + default: > > + return -EINVAL; > > + } > > + > > + if (i2s->slot_width) > > + lrck_period = i2s->slot_width; > > Here, i2s->slot_width is the number of bits for each slot, but in PCM mode, you you mean TDM here right? > need to multiply by the number of slots, like above. > > Also, there is already logic in sun4i_i2s_hw_params to use i2s->slot_width and > i2s->slots. You could avoid the duplication by passing slot_width/slots as > parameters to set_chan_cfg. Thanks for the catch, I will fix this. Regards, Clement > > Regards, > Samuel > > > + > > + regmap_update_bits(i2s->regmap, SUN4I_I2S_FMT0_REG, > > + SUN8I_I2S_FMT0_LRCK_PERIOD_MASK, > > + SUN8I_I2S_FMT0_LRCK_PERIOD(lrck_period)); > > + > > + regmap_update_bits(i2s->regmap, SUN8I_I2S_TX_CHAN_SEL_REG, > > + SUN50I_H6_I2S_TX_CHAN_EN_MASK, > > + SUN50I_H6_I2S_TX_CHAN_EN(channels)); > > + > > + return 0; > > +} > > + > > static int sun4i_i2s_hw_params(struct snd_pcm_substream *substream, > > struct snd_pcm_hw_params *params, > > struct snd_soc_dai *dai) > > [snip]