| Message ID | 20190814060854.26345-3-codekipper@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ASoC: sun4i-i2s: Updates to the driver | expand |
Hi, On Wed, Aug 14, 2019 at 08:08:41AM +0200, codekipper@gmail.com wrote: > From: Marcus Cooper <codekipper@gmail.com> > > Codecs without a control connection such as i2s based HDMI audio and > the Pine64 DAC require a different amount of bit clocks per frame than > what is calculated by the sample width. Use the tdm slot bindings to > provide this mechanism. > > Signed-off-by: Marcus Cooper <codekipper@gmail.com> > --- > sound/soc/sunxi/sun4i-i2s.c | 23 +++++++++++++++++++++-- > 1 file changed, 21 insertions(+), 2 deletions(-) > > diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c > index 8201334a059b..7c37b6291df0 100644 > --- a/sound/soc/sunxi/sun4i-i2s.c > +++ b/sound/soc/sunxi/sun4i-i2s.c > @@ -195,6 +195,9 @@ struct sun4i_i2s { > struct regmap_field *field_rxchansel; > > const struct sun4i_i2s_quirks *variant; > + > + unsigned int tdm_slots; > + unsigned int slot_width; > }; > > struct sun4i_i2s_clk_div { > @@ -346,7 +349,7 @@ static int sun4i_i2s_set_clk_rate(struct snd_soc_dai *dai, > if (i2s->variant->has_fmt_set_lrck_period) > regmap_update_bits(i2s->regmap, SUN4I_I2S_FMT0_REG, > SUN8I_I2S_FMT0_LRCK_PERIOD_MASK, > - SUN8I_I2S_FMT0_LRCK_PERIOD(32)); > + SUN8I_I2S_FMT0_LRCK_PERIOD(word_size)); > > > /* Set sign extension to pad out LSB with 0 */ > regmap_field_write(i2s->field_fmt_sext, 0); > @@ -450,7 +453,8 @@ static int sun4i_i2s_hw_params(struct snd_pcm_substream *substream, > regmap_field_write(i2s->field_fmt_sr, sr); > > return sun4i_i2s_set_clk_rate(dai, params_rate(params), > - params_width(params)); > + i2s->tdm_slots ? > + i2s->slot_width : params_width(params)); This is slightly more complicated than that. On the H3 (and all related ones), the CHAN_CFG_TX_SLOT_NUM and _RX_SLOT_NUM fields in the CHAN_CFG register need to be set to the number of slots as well. Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
On Wed, Aug 14, 2019 at 08:08:41AM +0200, codekipper@gmail.com wrote: > From: Marcus Cooper <codekipper@gmail.com> > > Codecs without a control connection such as i2s based HDMI audio and > the Pine64 DAC require a different amount of bit clocks per frame than This isn't a universal property of CODECs without a control, and it's something that CODECs with control can require too. > return sun4i_i2s_set_clk_rate(dai, params_rate(params), > - params_width(params)); > + i2s->tdm_slots ? > + i2s->slot_width : params_width(params)); Please write normal conditional statements unless there's a strong reason to do otherwise, it makes things more legible. > +static int sun4i_i2s_set_dai_tdm_slot(struct snd_soc_dai *dai, > + unsigned int tx_mask, > + unsigned int rx_mask, > + int slots, int width) > +{ > + struct sun4i_i2s *i2s = snd_soc_dai_get_drvdata(dai); > + > + i2s->tdm_slots = slots; > + > + i2s->slot_width = width; > + > + return 0; > +} No validation of the parameters here?
On Wed, 14 Aug 2019 at 11:30, Mark Brown <broonie@kernel.org> wrote: > > On Wed, Aug 14, 2019 at 08:08:41AM +0200, codekipper@gmail.com wrote: > > From: Marcus Cooper <codekipper@gmail.com> > > > > Codecs without a control connection such as i2s based HDMI audio and > > the Pine64 DAC require a different amount of bit clocks per frame than > > This isn't a universal property of CODECs without a control, and it's > something that CODECs with control can require too. ACK > > > return sun4i_i2s_set_clk_rate(dai, params_rate(params), > > - params_width(params)); > > + i2s->tdm_slots ? > > + i2s->slot_width : params_width(params)); > > Please write normal conditional statements unless there's a strong > reason to do otherwise, it makes things more legible. ACK > > > +static int sun4i_i2s_set_dai_tdm_slot(struct snd_soc_dai *dai, > > + unsigned int tx_mask, > > + unsigned int rx_mask, > > + int slots, int width) > > +{ > > + struct sun4i_i2s *i2s = snd_soc_dai_get_drvdata(dai); > > + > > + i2s->tdm_slots = slots; > > + > > + i2s->slot_width = width; > > + > > + return 0; > > +} > > No validation of the parameters here? ACK Thanks, CK
On Wed, 14 Aug 2019 at 13:08, Maxime Ripard <maxime.ripard@bootlin.com> wrote: > > Hi, > > On Wed, Aug 14, 2019 at 08:08:41AM +0200, codekipper@gmail.com wrote: > > From: Marcus Cooper <codekipper@gmail.com> > > > > Codecs without a control connection such as i2s based HDMI audio and > > the Pine64 DAC require a different amount of bit clocks per frame than > > what is calculated by the sample width. Use the tdm slot bindings to > > provide this mechanism. > > > > Signed-off-by: Marcus Cooper <codekipper@gmail.com> > > --- > > sound/soc/sunxi/sun4i-i2s.c | 23 +++++++++++++++++++++-- > > 1 file changed, 21 insertions(+), 2 deletions(-) > > > > diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c > > index 8201334a059b..7c37b6291df0 100644 > > --- a/sound/soc/sunxi/sun4i-i2s.c > > +++ b/sound/soc/sunxi/sun4i-i2s.c > > @@ -195,6 +195,9 @@ struct sun4i_i2s { > > struct regmap_field *field_rxchansel; > > > > const struct sun4i_i2s_quirks *variant; > > + > > + unsigned int tdm_slots; > > + unsigned int slot_width; > > }; > > > > struct sun4i_i2s_clk_div { > > @@ -346,7 +349,7 @@ static int sun4i_i2s_set_clk_rate(struct snd_soc_dai *dai, > > if (i2s->variant->has_fmt_set_lrck_period) > > regmap_update_bits(i2s->regmap, SUN4I_I2S_FMT0_REG, > > SUN8I_I2S_FMT0_LRCK_PERIOD_MASK, > > - SUN8I_I2S_FMT0_LRCK_PERIOD(32)); > > + SUN8I_I2S_FMT0_LRCK_PERIOD(word_size)); > > > > > > /* Set sign extension to pad out LSB with 0 */ > > regmap_field_write(i2s->field_fmt_sext, 0); > > @@ -450,7 +453,8 @@ static int sun4i_i2s_hw_params(struct snd_pcm_substream *substream, > > regmap_field_write(i2s->field_fmt_sr, sr); > > > > return sun4i_i2s_set_clk_rate(dai, params_rate(params), > > - params_width(params)); > > + i2s->tdm_slots ? > > + i2s->slot_width : params_width(params)); > > This is slightly more complicated than that. At this point we're only supporting 2 channels with fixed slot settings. I've added a comment to state that we're using the tdm_slot at the moment as an indicator to override the slot width. Do you think that is enough for now?. Thanks, CK > > On the H3 (and all related ones), the CHAN_CFG_TX_SLOT_NUM and > _RX_SLOT_NUM fields in the CHAN_CFG register need to be set to the > number of slots as well. > > Maxime > > -- > Maxime Ripard, Bootlin > Embedded Linux and Kernel engineering > https://bootlin.com
diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c index 8201334a059b..7c37b6291df0 100644 --- a/sound/soc/sunxi/sun4i-i2s.c +++ b/sound/soc/sunxi/sun4i-i2s.c @@ -195,6 +195,9 @@ struct sun4i_i2s { struct regmap_field *field_rxchansel; const struct sun4i_i2s_quirks *variant; + + unsigned int tdm_slots; + unsigned int slot_width; }; struct sun4i_i2s_clk_div { @@ -346,7 +349,7 @@ static int sun4i_i2s_set_clk_rate(struct snd_soc_dai *dai, if (i2s->variant->has_fmt_set_lrck_period) regmap_update_bits(i2s->regmap, SUN4I_I2S_FMT0_REG, SUN8I_I2S_FMT0_LRCK_PERIOD_MASK, - SUN8I_I2S_FMT0_LRCK_PERIOD(32)); + SUN8I_I2S_FMT0_LRCK_PERIOD(word_size)); /* Set sign extension to pad out LSB with 0 */ regmap_field_write(i2s->field_fmt_sext, 0); @@ -450,7 +453,8 @@ static int sun4i_i2s_hw_params(struct snd_pcm_substream *substream, regmap_field_write(i2s->field_fmt_sr, sr); return sun4i_i2s_set_clk_rate(dai, params_rate(params), - params_width(params)); + i2s->tdm_slots ? + i2s->slot_width : params_width(params)); } static int sun4i_i2s_set_fmt(struct snd_soc_dai *dai, unsigned int fmt) @@ -693,10 +697,25 @@ static int sun4i_i2s_set_sysclk(struct snd_soc_dai *dai, int clk_id, return 0; } +static int sun4i_i2s_set_dai_tdm_slot(struct snd_soc_dai *dai, + unsigned int tx_mask, + unsigned int rx_mask, + int slots, int width) +{ + struct sun4i_i2s *i2s = snd_soc_dai_get_drvdata(dai); + + i2s->tdm_slots = slots; + + i2s->slot_width = width; + + return 0; +} + static const struct snd_soc_dai_ops sun4i_i2s_dai_ops = { .hw_params = sun4i_i2s_hw_params, .set_fmt = sun4i_i2s_set_fmt, .set_sysclk = sun4i_i2s_set_sysclk, + .set_tdm_slot = sun4i_i2s_set_dai_tdm_slot, .trigger = sun4i_i2s_trigger, };