| Message ID | 20230221183211.21964-5-clamor95@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Fix sound on ASUS Transformers | expand |
On Tue, Feb 21, 2023 at 08:32:05PM +0200, Svyatoslav Ryhel wrote: > diff --git a/sound/soc/tegra/tegra_asoc_machine.c b/sound/soc/tegra/tegra_asoc_machine.c > index 78faa8bcae27..607800ec07a6 100644 > --- a/sound/soc/tegra/tegra_asoc_machine.c > +++ b/sound/soc/tegra/tegra_asoc_machine.c > @@ -51,6 +51,17 @@ static struct snd_soc_jack_gpio tegra_machine_headset_jack_gpio = { > }; > > /* Mic Jack */ This comment doesn't make sense now. It was never super useful, though. Just delete it. > +static int headset_check(void *data) > +{ > + struct tegra_machine *machine = (struct tegra_machine *)data; > + > + /* Detect mic insertion only if 3.5 jack is in */ > + if (gpiod_get_value_cansleep(machine->gpiod_hp_det) && > + gpiod_get_value_cansleep(machine->gpiod_mic_det)) > + return SND_JACK_MICROPHONE; > + > + return 0; > +} > > static struct snd_soc_jack tegra_machine_mic_jack; > > @@ -183,8 +194,15 @@ int tegra_asoc_machine_init(struct snd_soc_pcm_runtime *rtd) regards, dan carpenter
On Tue, Feb 21, 2023 at 08:32:05PM +0200, Svyatoslav Ryhel wrote: > Add Realtek ALC5631/RT5631 codec support to the Tegra ASoC machine driver. > The RT5631 codec is found on devices like ASUS Transformer TF201, TF700T > and other Tegra-based Android tablets. > > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com> > Signed-off-by: Ion Agorria <ion@agorria.com> Your signoff should be last if you're the one sending this. > +static unsigned int tegra_machine_mclk_rate_6mhz(unsigned int srate) > +{ > + unsigned int mclk; > + > + switch (srate) { > + case 64000: > + case 88200: > + case 96000: > + mclk = 128 * srate; > + break; > + default: > + mclk = 256 * srate; > + break; > + } > + /* FIXME: Codec only requires >= 3MHz if OSR==0 */ > + while (mclk < 6000000) > + mclk *= 2; It feels like this is complicated enough and looks like the clocking is flexible enough that it might be easier to just have a table of values or otherwise enumerate standard rates, seeing the code I feel like I need to worry about what happens if we pick a clock rate over 6MHz (the loop could give a value over that), and it's not clear why we have the switch statement rather than just starting at a multiple of 128 and looping an extra time. I suspect there's going to be no meaningful downside for having the clock held at over 3MHz on a tablet form factor, the usual issue would be power consumption but between the larger battery size you tend to have on a tablet and the power draw of the screen if that's on it's likely to be into the noise practially speaking.
вт, 21 лют. 2023 р. о 21:32 Dan Carpenter <error27@gmail.com> пише: > > On Tue, Feb 21, 2023 at 08:32:05PM +0200, Svyatoslav Ryhel wrote: > > diff --git a/sound/soc/tegra/tegra_asoc_machine.c b/sound/soc/tegra/tegra_asoc_machine.c > > index 78faa8bcae27..607800ec07a6 100644 > > --- a/sound/soc/tegra/tegra_asoc_machine.c > > +++ b/sound/soc/tegra/tegra_asoc_machine.c > > @@ -51,6 +51,17 @@ static struct snd_soc_jack_gpio tegra_machine_headset_jack_gpio = { > > }; > > > > /* Mic Jack */ > > This comment doesn't make sense now. It was never super useful, though. > Just delete it. It does. Headset is Mic Jack + Headphones combined. headset_check function performs check for a Mic Jack component in plugged Jack 3.5 > > +static int headset_check(void *data) > > +{ > > + struct tegra_machine *machine = (struct tegra_machine *)data; > > + > > + /* Detect mic insertion only if 3.5 jack is in */ > > + if (gpiod_get_value_cansleep(machine->gpiod_hp_det) && > > + gpiod_get_value_cansleep(machine->gpiod_mic_det)) > > + return SND_JACK_MICROPHONE; > > + > > + return 0; > > +} > > > > static struct snd_soc_jack tegra_machine_mic_jack; > > > > @@ -183,8 +194,15 @@ int tegra_asoc_machine_init(struct snd_soc_pcm_runtime *rtd) > > regards, > dan carpenter
ср, 22 лют. 2023 р. о 00:23 Mark Brown <broonie@kernel.org> пише: > > On Tue, Feb 21, 2023 at 08:32:05PM +0200, Svyatoslav Ryhel wrote: > > > Add Realtek ALC5631/RT5631 codec support to the Tegra ASoC machine driver. > > The RT5631 codec is found on devices like ASUS Transformer TF201, TF700T > > and other Tegra-based Android tablets. > > > > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com> > > Signed-off-by: Ion Agorria <ion@agorria.com> > > Your signoff should be last if you're the one sending this. Thanks > > +static unsigned int tegra_machine_mclk_rate_6mhz(unsigned int srate) > > +{ > > + unsigned int mclk; > > + > > + switch (srate) { > > + case 64000: > > + case 88200: > > + case 96000: > > + mclk = 128 * srate; > > + break; > > + default: > > + mclk = 256 * srate; > > + break; > > + } > > + /* FIXME: Codec only requires >= 3MHz if OSR==0 */ > > + while (mclk < 6000000) > > + mclk *= 2; > > It feels like this is complicated enough and looks like the > clocking is flexible enough that it might be easier to just have > a table of values or otherwise enumerate standard rates, seeing > the code I feel like I need to worry about what happens if we > pick a clock rate over 6MHz (the loop could give a value over > that), and it's not clear why we have the switch statement rather > than just starting at a multiple of 128 and looping an extra time. > > I suspect there's going to be no meaningful downside for having > the clock held at over 3MHz on a tablet form factor, the usual > issue would be power consumption but between the larger battery > size you tend to have on a tablet and the power draw of the > screen if that's on it's likely to be into the noise practially > speaking. This is how downstream handled mclk rate for RT5631.
On Wed, Feb 22, 2023 at 10:00:58AM +0200, Svyatoslav Ryhel wrote: > ср, 22 лют. 2023 р. о 00:23 Mark Brown <broonie@kernel.org> пише: > > It feels like this is complicated enough and looks like the > > clocking is flexible enough that it might be easier to just have > > a table of values or otherwise enumerate standard rates, seeing > > the code I feel like I need to worry about what happens if we > > pick a clock rate over 6MHz (the loop could give a value over > > that), and it's not clear why we have the switch statement rather > > than just starting at a multiple of 128 and looping an extra time. > > I suspect there's going to be no meaningful downside for having > > the clock held at over 3MHz on a tablet form factor, the usual > > issue would be power consumption but between the larger battery > > size you tend to have on a tablet and the power draw of the > > screen if that's on it's likely to be into the noise practially > > speaking. > This is how downstream handled mclk rate for RT5631. That doesn't mean it shouldn't be fixed or improved.
On Wed, Feb 22, 2023 at 09:55:52AM +0200, Svyatoslav Ryhel wrote: > вт, 21 лют. 2023 р. о 21:32 Dan Carpenter <error27@gmail.com> пише: > > > > On Tue, Feb 21, 2023 at 08:32:05PM +0200, Svyatoslav Ryhel wrote: > > > diff --git a/sound/soc/tegra/tegra_asoc_machine.c b/sound/soc/tegra/tegra_asoc_machine.c > > > index 78faa8bcae27..607800ec07a6 100644 > > > --- a/sound/soc/tegra/tegra_asoc_machine.c > > > +++ b/sound/soc/tegra/tegra_asoc_machine.c > > > @@ -51,6 +51,17 @@ static struct snd_soc_jack_gpio tegra_machine_headset_jack_gpio = { > > > }; > > > > > > /* Mic Jack */ > > > > This comment doesn't make sense now. It was never super useful, though. > > Just delete it. > > It does. Headset is Mic Jack + Headphones combined. headset_check function > performs check for a Mic Jack component in plugged Jack 3.5 > I feel if we need to discuess what a comment means or if it even means anything then that's a useless comment by definition. regards, dan carpenter
On Wed, Feb 22, 2023 at 04:28:09PM +0300, Dan Carpenter wrote: > On Wed, Feb 22, 2023 at 09:55:52AM +0200, Svyatoslav Ryhel wrote: > > вт, 21 лют. 2023 р. о 21:32 Dan Carpenter <error27@gmail.com> пише: > > > > /* Mic Jack */ > > > This comment doesn't make sense now. It was never super useful, though. > > > Just delete it. > > It does. Headset is Mic Jack + Headphones combined. headset_check function > > performs check for a Mic Jack component in plugged Jack 3.5 > I feel if we need to discuess what a comment means or if it even means > anything then that's a useless comment by definition. If the device doesn't have a distinct mic jack then it's not ideal to talk about there being one (as opposed to the microphone on the headset jack).
diff --git a/sound/soc/tegra/Kconfig b/sound/soc/tegra/Kconfig index b6712a3d1fa1..ff905e5dcd86 100644 --- a/sound/soc/tegra/Kconfig +++ b/sound/soc/tegra/Kconfig @@ -189,6 +189,15 @@ config SND_SOC_TEGRA_AUDIO_GRAPH_CARD config SND_SOC_TEGRA_MACHINE_DRV tristate +config SND_SOC_TEGRA_RT5631 + tristate "SoC Audio support for Tegra boards using an RT5631 codec" + depends on SND_SOC_TEGRA && I2C && GPIOLIB + select SND_SOC_TEGRA_MACHINE_DRV + select SND_SOC_RT5631 + help + Say Y or M here if you want to add support for SoC audio on Tegra + boards using the RT5631 codec, such as Transformer. + config SND_SOC_TEGRA_RT5640 tristate "SoC Audio support for Tegra boards using an RT5640 codec" depends on I2C && GPIOLIB diff --git a/sound/soc/tegra/tegra_asoc_machine.c b/sound/soc/tegra/tegra_asoc_machine.c index 78faa8bcae27..607800ec07a6 100644 --- a/sound/soc/tegra/tegra_asoc_machine.c +++ b/sound/soc/tegra/tegra_asoc_machine.c @@ -51,6 +51,17 @@ static struct snd_soc_jack_gpio tegra_machine_headset_jack_gpio = { }; /* Mic Jack */ +static int headset_check(void *data) +{ + struct tegra_machine *machine = (struct tegra_machine *)data; + + /* Detect mic insertion only if 3.5 jack is in */ + if (gpiod_get_value_cansleep(machine->gpiod_hp_det) && + gpiod_get_value_cansleep(machine->gpiod_mic_det)) + return SND_JACK_MICROPHONE; + + return 0; +} static struct snd_soc_jack tegra_machine_mic_jack; @@ -183,8 +194,15 @@ int tegra_asoc_machine_init(struct snd_soc_pcm_runtime *rtd) return err; } + tegra_machine_mic_jack_gpio.data = machine; tegra_machine_mic_jack_gpio.desc = machine->gpiod_mic_det; + if (of_property_read_bool(card->dev->of_node, + "nvidia,coupled-mic-hp-det")) { + tegra_machine_mic_jack_gpio.desc = machine->gpiod_hp_det; + tegra_machine_mic_jack_gpio.jack_status_check = headset_check; + }; + err = snd_soc_jack_add_gpios(&tegra_machine_mic_jack, 1, &tegra_machine_mic_jack_gpio); if (err) @@ -238,6 +256,27 @@ static unsigned int tegra_machine_mclk_rate_12mhz(unsigned int srate) return mclk; } +static unsigned int tegra_machine_mclk_rate_6mhz(unsigned int srate) +{ + unsigned int mclk; + + switch (srate) { + case 64000: + case 88200: + case 96000: + mclk = 128 * srate; + break; + default: + mclk = 256 * srate; + break; + } + /* FIXME: Codec only requires >= 3MHz if OSR==0 */ + while (mclk < 6000000) + mclk *= 2; + + return mclk; +} + static int tegra_machine_hw_params(struct snd_pcm_substream *substream, struct snd_pcm_hw_params *params) { @@ -865,6 +904,40 @@ static const struct tegra_asoc_data tegra_rt5632_data = { .add_headset_jack = true, }; +/* RT5631 machine */ + +SND_SOC_DAILINK_DEFS(rt5631_hifi, + DAILINK_COMP_ARRAY(COMP_EMPTY()), + DAILINK_COMP_ARRAY(COMP_CODEC(NULL, "rt5631-hifi")), + DAILINK_COMP_ARRAY(COMP_EMPTY())); + +static struct snd_soc_dai_link tegra_rt5631_dai = { + .name = "RT5631", + .stream_name = "RT5631 PCM", + .init = tegra_asoc_machine_init, + .dai_fmt = SND_SOC_DAIFMT_I2S | + SND_SOC_DAIFMT_NB_NF | + SND_SOC_DAIFMT_CBS_CFS, + SND_SOC_DAILINK_REG(rt5631_hifi), +}; + +static struct snd_soc_card snd_soc_tegra_rt5631 = { + .components = "codec:rt5631", + .dai_link = &tegra_rt5631_dai, + .num_links = 1, + .fully_routed = true, +}; + +static const struct tegra_asoc_data tegra_rt5631_data = { + .mclk_rate = tegra_machine_mclk_rate_6mhz, + .card = &snd_soc_tegra_rt5631, + .add_common_dapm_widgets = true, + .add_common_controls = true, + .add_common_snd_ops = true, + .add_mic_jack = true, + .add_hp_jack = true, +}; + static const struct of_device_id tegra_machine_of_match[] = { { .compatible = "nvidia,tegra-audio-trimslice", .data = &tegra_trimslice_data }, { .compatible = "nvidia,tegra-audio-max98090", .data = &tegra_max98090_data }, @@ -874,6 +947,7 @@ static const struct of_device_id tegra_machine_of_match[] = { { .compatible = "nvidia,tegra-audio-rt5677", .data = &tegra_rt5677_data }, { .compatible = "nvidia,tegra-audio-rt5640", .data = &tegra_rt5640_data }, { .compatible = "nvidia,tegra-audio-alc5632", .data = &tegra_rt5632_data }, + { .compatible = "nvidia,tegra-audio-rt5631", .data = &tegra_rt5631_data }, {}, }; MODULE_DEVICE_TABLE(of, tegra_machine_of_match);