[alsa-devel] [PATCH RFC v3 0/8] Beaglebone-Black HDMI audio
Changes since RFC v2 version of the patches: - Dropped out already applied: ASoC: hdmi-codec: Add devicetree binding with documentation - Addresses Mark's comments here: http://mailman.alsa-project.org/pipermail/alsa-devel/2013-December/070605.ht... - ASoC: davinci-evm: Add named clock reference to DT bindings - Get rid of unnecessary castings - Add mclk NULL checks - Use devm_clk_get() - Change clock name from "ti,codec-clock" to "mclk" - Address Mark's comments here: http://mailman.alsa-project.org/pipermail/alsa-devel/2013-December/070606.ht... - ASoC: davinci-evm: HDMI audio support for TDA998x trough McASP I2S bus - Get rid of unnecessary castings - Update commit message - Add: ASoC: davinci-mcasp: Set BCLK divider if McASP is BCLK master - Use snd_soc_params_to_bclk()
Changes since the first RFC version of the patches: - Drop out already applied: ASoC: hdmi-codec: Add SNDRV_PCM_FMTBIT_32_LE playback format - Change sound node's compatible property form: "ti,am33xx-beaglebone-black" to "ti,am33xx-beaglebone-black-audio" - Some minor style issue fixes from TI internal review
Jyri Sarha (8): clk: add gpio controlled clock ASoC: davinci-evm: Add named clock reference to DT bindings ASoC: davinci-mcasp: Set BCLK divider if McASP is BCLK master ASoC: davinci-evm: HDMI audio support for TDA998x trough McASP I2S bus ASoC: davinci: HDMI audio build for AM33XX and TDA998x drm/tilcdc: Add I2C HDMI audio config for tda998x ARM: OMAP2+: omap2plus_defconfig: Enable tilcdc and TDA998X HDMI support ARM: OMAP2+: omap2plus_defconfig: Enable BeagleBone Black HDMI audio support
.../devicetree/bindings/clock/gpio-clock.txt | 21 ++ .../bindings/sound/davinci-evm-audio.txt | 13 +- arch/arm/configs/omap2plus_defconfig | 5 + drivers/clk/Makefile | 1 + drivers/clk/clk-gpio.c | 210 +++++++++++++++++++ drivers/gpu/drm/tilcdc/tilcdc_slave.c | 24 ++- include/linux/clk-provider.h | 25 +++ sound/soc/davinci/Kconfig | 12 ++ sound/soc/davinci/Makefile | 1 + sound/soc/davinci/davinci-evm.c | 211 +++++++++++++++++++- sound/soc/davinci/davinci-mcasp.c | 20 ++ 11 files changed, 534 insertions(+), 9 deletions(-) create mode 100644 Documentation/devicetree/bindings/clock/gpio-clock.txt create mode 100644 drivers/clk/clk-gpio.c
The added clk-gpio is a basic clock that can be enabled and disabled trough a gpio output. The DT binding document for the clock is also added. For EPROBE_DEFER handling the registering of the clock has to be delayed until of_clk_get() call time.
Signed-off-by: Jyri Sarha jsarha@ti.com cc: mturquette@linaro.org cc: bcousson@baylibre.com --- .../devicetree/bindings/clock/gpio-clock.txt | 21 ++ drivers/clk/Makefile | 1 + drivers/clk/clk-gpio.c | 210 ++++++++++++++++++++ include/linux/clk-provider.h | 25 +++ 4 files changed, 257 insertions(+) create mode 100644 Documentation/devicetree/bindings/clock/gpio-clock.txt create mode 100644 drivers/clk/clk-gpio.c
diff --git a/Documentation/devicetree/bindings/clock/gpio-clock.txt b/Documentation/devicetree/bindings/clock/gpio-clock.txt new file mode 100644 index 0000000..54fea39 --- /dev/null +++ b/Documentation/devicetree/bindings/clock/gpio-clock.txt @@ -0,0 +1,21 @@ +Binding for simple gpio controlled clock. + +This binding uses the common clock binding[1]. + +[1] Documentation/devicetree/bindings/clock/clock-bindings.txt + +Required properties: +- compatible : shall be "gpio-clock". +- #clock-cells : from common clock binding; shall be set to 0. +- enable-gpios : GPIO reference for enabling and disabling the clock. + +Optional properties: +- clocks: Maximum of one parent clock is supported. + +Example: + clock { + compatible = "gpio-clock"; + clocks = <&parentclk>; + #clock-cells = <0>; + enable-gpios = <&gpio 1 GPIO_ACTIVE_HIGH>; + }; diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile index 7a10bc9..9616e3a 100644 --- a/drivers/clk/Makefile +++ b/drivers/clk/Makefile @@ -8,6 +8,7 @@ obj-$(CONFIG_COMMON_CLK) += clk-fixed-rate.o obj-$(CONFIG_COMMON_CLK) += clk-gate.o obj-$(CONFIG_COMMON_CLK) += clk-mux.o obj-$(CONFIG_COMMON_CLK) += clk-composite.o +obj-$(CONFIG_COMMON_CLK) += clk-gpio.o
# SoCs specific obj-$(CONFIG_ARCH_BCM2835) += clk-bcm2835.o diff --git a/drivers/clk/clk-gpio.c b/drivers/clk/clk-gpio.c new file mode 100644 index 0000000..e04b0e1 --- /dev/null +++ b/drivers/clk/clk-gpio.c @@ -0,0 +1,210 @@ +/* + * Copyright (C) 2013 Texas Instruments + * Author: Jyri Sarha jsarha@ti.com + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * Gpio controlled clock implementation + */ + +#include <linux/clk-provider.h> +#include <linux/module.h> +#include <linux/slab.h> +#include <linux/gpio.h> +#include <linux/of_gpio.h> +#include <linux/err.h> +#include <linux/device.h> + +/** + * DOC: basic gpio controlled clock which can be enabled and disabled + * with gpio output + * Traits of this clock: + * prepare - clk_(un)prepare only ensures parent is (un)prepared + * enable - clk_enable and clk_disable are functional & control gpio + * rate - inherits rate from parent. No clk_set_rate support + * parent - fixed parent. No clk_set_parent support + */ + +#define to_clk_gpio(_hw) container_of(_hw, struct clk_gpio, hw) + +static int clk_gpio_enable(struct clk_hw *hw) +{ + struct clk_gpio *gpio = to_clk_gpio(hw); + int value = gpio->active_low ? 0 : 1; + + gpio_set_value(gpio->gpio, value); + + return 0; +} + +static void clk_gpio_disable(struct clk_hw *hw) +{ + struct clk_gpio *gpio = to_clk_gpio(hw); + int value = gpio->active_low ? 1 : 0; + + gpio_set_value(gpio->gpio, value); +} + +static int clk_gpio_is_enabled(struct clk_hw *hw) +{ + struct clk_gpio *gpio = to_clk_gpio(hw); + int value = gpio_get_value(gpio->gpio); + + return gpio->active_low ? !value : value; +} + +const struct clk_ops clk_gpio_ops = { + .enable = clk_gpio_enable, + .disable = clk_gpio_disable, + .is_enabled = clk_gpio_is_enabled, +}; +EXPORT_SYMBOL_GPL(clk_gpio_ops); + +/** + * clk_register_gpio - register a gpip clock with the clock framework + * @dev: device that is registering this clock + * @name: name of this clock + * @parent_name: name of this clock's parent + * @flags: framework-specific flags for this clock + * @gpio: gpio to control this clock + * @active_low: gpio polarity + */ +struct clk *clk_register_gpio(struct device *dev, const char *name, + const char *parent_name, unsigned long flags, + unsigned int gpio, bool active_low) +{ + struct clk_gpio *clk_gpio; + struct clk *clk = ERR_PTR(-EINVAL); + struct clk_init_data init = { NULL }; + unsigned long gpio_flags; + int err; + + if (active_low) + gpio_flags = GPIOF_OUT_INIT_LOW; + else + gpio_flags = GPIOF_OUT_INIT_HIGH; + + err = gpio_request_one(gpio, gpio_flags, name); + + if (err) { + pr_err("%s: %s: Error requesting clock control gpio %u\n", + __func__, name, gpio); + clk = ERR_PTR(err); + goto clk_register_gpio_err; + } + + clk_gpio = kzalloc(sizeof(*clk_gpio), GFP_KERNEL); + + if (!clk_gpio) { + pr_err("%s: %s: could not allocate gpio clk\n", __func__, name); + clk = ERR_PTR(-ENOMEM); + goto clk_register_gpio_err; + } + + init.name = name; + init.ops = &clk_gpio_ops; + init.flags = flags | CLK_IS_BASIC; + init.parent_names = (parent_name ? &parent_name : NULL); + init.num_parents = (parent_name ? 1 : 0); + + clk_gpio->gpio = gpio; + clk_gpio->active_low = active_low; + clk_gpio->hw.init = &init; + + clk = clk_register(dev, &clk_gpio->hw); + + if (!IS_ERR(clk)) + return clk; + + kfree(clk_gpio); + +clk_register_gpio_err: + gpio_free(gpio); + + return clk; +} +EXPORT_SYMBOL_GPL(clk_register_gpio); + +#ifdef CONFIG_OF +/** + * The clk_register_gpio has to be delayed, because the EPROBE_DEFER + * can not be handled properly at of_clk_init() call time. + */ + +struct clk_gpio_delayed_register_data { + struct device_node *node; + struct mutex lock; /* Protect delayed clk registering */ + struct clk *clk; +}; + +static +struct clk *of_clk_gpio_delayed_register_get(struct of_phandle_args *clkspec, + void *_data) +{ + struct clk_gpio_delayed_register_data *data = + (struct clk_gpio_delayed_register_data *)_data; + struct clk *clk; + const char *clk_name = data->node->name; + const char *parent_name; + enum of_gpio_flags gpio_flags; + int gpio; + bool active_low; + + mutex_lock(&data->lock); + + if (data->clk) { + mutex_unlock(&data->lock); + return data->clk; + } + + gpio = of_get_named_gpio_flags(data->node, "enable-gpios", 0, + &gpio_flags); + + if (gpio < 0) { + mutex_unlock(&data->lock); + if (gpio != -EPROBE_DEFER) + pr_err("%s: %s: Can't get 'enable-gpios' DT property\n", + __func__, clk_name); + return ERR_PTR(gpio); + } + + active_low = gpio_flags & OF_GPIO_ACTIVE_LOW; + + parent_name = of_clk_get_parent_name(data->node, 0); + + clk = clk_register_gpio(NULL, clk_name, parent_name, 0, + gpio, active_low); + if (IS_ERR(clk)) { + mutex_unlock(&data->lock); + return clk; + } + + data->clk = clk; + mutex_unlock(&data->lock); + + return clk; +} + +/** + * of_gpio_clk_setup() - Setup function for gpio controlled clock + */ +void __init of_gpio_clk_setup(struct device_node *node) +{ + struct clk_gpio_delayed_register_data *data; + + data = kzalloc(sizeof(*data), GFP_KERNEL); + if (!data) { + pr_err("%s: could not allocate gpio clk\n", __func__); + return; + } + + data->node = node; + mutex_init(&data->lock); + + of_clk_add_provider(node, of_clk_gpio_delayed_register_get, data); +} +EXPORT_SYMBOL_GPL(of_gpio_clk_setup); +CLK_OF_DECLARE(gpio_clk, "gpio-clock", of_gpio_clk_setup); +#endif diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h index 7e59253..21082b2 100644 --- a/include/linux/clk-provider.h +++ b/include/linux/clk-provider.h @@ -407,6 +407,31 @@ struct clk *clk_register_composite(struct device *dev, const char *name, struct clk_hw *gate_hw, const struct clk_ops *gate_ops, unsigned long flags);
+/*** + * struct clk_gpio - gpio controlled clock + * + * @hw: handle between common and hardware-specific interfaces + * @gpio: gpio + * @active_low: gpio polarity + * + * Clock with a gpio control for enabling and disabling the parent clock. + * Implements .enable, .disable and .is_enabled + */ + +struct clk_gpio { + struct clk_hw hw; + unsigned int gpio; + bool active_low; +}; + +extern const struct clk_ops clk_gpio_ops; + +struct clk *clk_register_gpio(struct device *dev, const char *name, + const char *parent_name, unsigned long flags, + unsigned int gpio, bool active_low); + +void of_gpio_clk_setup(struct device_node *node); + /** * clk_register - allocate a new clock, register it and return an opaque cookie * @dev: device that is registering this clock
The referenced clock is used to get codec clock rate and the clock is disabled and enabled in startup and shutdown snd_soc_ops call backs. The change is also documented in DT bindigs document.
Signed-off-by: Jyri Sarha jsarha@ti.com cc: bcousson@baylibre.com --- .../bindings/sound/davinci-evm-audio.txt | 9 ++- sound/soc/davinci/davinci-evm.c | 58 +++++++++++++++++++- 2 files changed, 64 insertions(+), 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/sound/davinci-evm-audio.txt b/Documentation/devicetree/bindings/sound/davinci-evm-audio.txt index 865178d..963e100 100644 --- a/Documentation/devicetree/bindings/sound/davinci-evm-audio.txt +++ b/Documentation/devicetree/bindings/sound/davinci-evm-audio.txt @@ -5,12 +5,19 @@ Required properties: - ti,model : The user-visible name of this sound complex. - ti,audio-codec : The phandle of the TLV320AIC3x audio codec - ti,mcasp-controller : The phandle of the McASP controller -- ti,codec-clock-rate : The Codec Clock rate (in Hz) applied to the Codec - ti,audio-routing : A list of the connections between audio components. Each entry is a pair of strings, the first being the connection's sink, the second being the connection's source. Valid names for sources and sinks are the codec's pins, and the jacks on the board:
+Optional properties: +- ti,codec-clock-rate : The Codec Clock rate (in Hz) applied to the Codec. +- clocks : Reference to the master clock +- clock-names : The clock should be named "mclk" +- Either codec-clock-rate or the codec-clock reference has to be defined. If + the both are defined the driver attempts to set referenced clock to the + defined rate and takes the rate from the clock reference. + Board connectors:
* Headphone Jack diff --git a/sound/soc/davinci/davinci-evm.c b/sound/soc/davinci/davinci-evm.c index 70ff377..d3e4cb0 100644 --- a/sound/soc/davinci/davinci-evm.c +++ b/sound/soc/davinci/davinci-evm.c @@ -17,6 +17,7 @@ #include <linux/platform_data/edma.h> #include <linux/i2c.h> #include <linux/of_platform.h> +#include <linux/clk.h> #include <sound/core.h> #include <sound/pcm.h> #include <sound/soc.h> @@ -30,9 +31,34 @@ #include "davinci-i2s.h"
struct snd_soc_card_drvdata_davinci { + struct clk *mclk; unsigned sysclk; };
+static int evm_startup(struct snd_pcm_substream *substream) +{ + struct snd_soc_pcm_runtime *rtd = substream->private_data; + struct snd_soc_card *soc_card = rtd->codec->card; + struct snd_soc_card_drvdata_davinci *drvdata = + snd_soc_card_get_drvdata(soc_card); + + if (drvdata->mclk) + return clk_prepare_enable(drvdata->mclk); + + return 0; +} + +static void evm_shutdown(struct snd_pcm_substream *substream) +{ + struct snd_soc_pcm_runtime *rtd = substream->private_data; + struct snd_soc_card *soc_card = rtd->codec->card; + struct snd_soc_card_drvdata_davinci *drvdata = + snd_soc_card_get_drvdata(soc_card); + + if (drvdata->mclk) + clk_disable_unprepare(drvdata->mclk); +} + static int evm_hw_params(struct snd_pcm_substream *substream, struct snd_pcm_hw_params *params) { @@ -59,6 +85,8 @@ static int evm_hw_params(struct snd_pcm_substream *substream, }
static struct snd_soc_ops evm_ops = { + .startup = evm_startup, + .shutdown = evm_shutdown, .hw_params = evm_hw_params, };
@@ -348,6 +376,7 @@ static int davinci_evm_probe(struct platform_device *pdev) of_match_device(of_match_ptr(davinci_evm_dt_ids), &pdev->dev); struct snd_soc_dai_link *dai = (struct snd_soc_dai_link *) match->data; struct snd_soc_card_drvdata_davinci *drvdata = NULL; + struct clk *mclk; int ret = 0;
evm_soc_card.dai_link = dai; @@ -367,13 +396,38 @@ static int davinci_evm_probe(struct platform_device *pdev) if (ret) return ret;
+ mclk = devm_clk_get(&pdev->dev, "mclk"); + if (PTR_ERR(mclk) == -EPROBE_DEFER) { + return -EPROBE_DEFER; + } else if (IS_ERR(mclk)) { + dev_dbg(&pdev->dev, "mclk not found.\n"); + mclk = NULL; + } + drvdata = devm_kzalloc(&pdev->dev, sizeof(*drvdata), GFP_KERNEL); if (!drvdata) return -ENOMEM;
+ drvdata->mclk = mclk; + ret = of_property_read_u32(np, "ti,codec-clock-rate", &drvdata->sysclk); - if (ret < 0) - return -EINVAL; + + if (ret < 0) { + if (!drvdata->mclk) { + dev_err(&pdev->dev, + "No clock or clock rate defined.\n"); + return -EINVAL; + } + drvdata->sysclk = clk_get_rate(drvdata->mclk); + } else if (drvdata->mclk) { + unsigned int requestd_rate = drvdata->sysclk; + clk_set_rate(drvdata->mclk, drvdata->sysclk); + drvdata->sysclk = clk_get_rate(drvdata->mclk); + if (drvdata->sysclk != requestd_rate) + dev_warn(&pdev->dev, + "Could not get requested rate %u using %u.\n", + requestd_rate, drvdata->sysclk); + }
snd_soc_card_set_drvdata(&evm_soc_card, drvdata); ret = devm_snd_soc_register_card(&pdev->dev, &evm_soc_card);
On Mon, Jan 27, 2014 at 05:37:51PM +0200, Jyri Sarha wrote:
The referenced clock is used to get codec clock rate and the clock is disabled and enabled in startup and shutdown snd_soc_ops call backs. The change is also documented in DT bindigs document.
Applied, thanks.
Make BCLK divider setting implicite in hw_params call if McASP device is the bit clock master on the audio serial bus.
Signed-off-by: Jyri Sarha jsarha@ti.com --- sound/soc/davinci/davinci-mcasp.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)
diff --git a/sound/soc/davinci/davinci-mcasp.c b/sound/soc/davinci/davinci-mcasp.c index b7858bf..ae328be 100644 --- a/sound/soc/davinci/davinci-mcasp.c +++ b/sound/soc/davinci/davinci-mcasp.c @@ -53,6 +53,9 @@ struct davinci_mcasp { u16 bclk_lrclk_ratio; int streams;
+ int sysclk_freq; + bool bclk_master; + /* McASP FIFO related */ u8 txnumevt; u8 rxnumevt; @@ -292,6 +295,7 @@ static int davinci_mcasp_set_dai_fmt(struct snd_soc_dai *cpu_dai,
mcasp_set_bits(mcasp, DAVINCI_MCASP_PDIR_REG, ACLKX | ACLKR); mcasp_set_bits(mcasp, DAVINCI_MCASP_PDIR_REG, AFSX | AFSR); + mcasp->bclk_master = 1; break; case SND_SOC_DAIFMT_CBM_CFS: /* codec is clock master and frame slave */ @@ -303,6 +307,7 @@ static int davinci_mcasp_set_dai_fmt(struct snd_soc_dai *cpu_dai,
mcasp_clr_bits(mcasp, DAVINCI_MCASP_PDIR_REG, ACLKX | ACLKR); mcasp_set_bits(mcasp, DAVINCI_MCASP_PDIR_REG, AFSX | AFSR); + mcasp->bclk_master = 0; break; case SND_SOC_DAIFMT_CBM_CFM: /* codec is clock and frame master */ @@ -314,6 +319,7 @@ static int davinci_mcasp_set_dai_fmt(struct snd_soc_dai *cpu_dai,
mcasp_clr_bits(mcasp, DAVINCI_MCASP_PDIR_REG, ACLKX | AHCLKX | AFSX | ACLKR | AHCLKR | AFSR); + mcasp->bclk_master = 0; break;
default: @@ -405,6 +411,8 @@ static int davinci_mcasp_set_sysclk(struct snd_soc_dai *dai, int clk_id, mcasp_clr_bits(mcasp, DAVINCI_MCASP_PDIR_REG, AHCLKX); }
+ mcasp->sysclk_freq = freq; + return 0; }
@@ -607,6 +615,18 @@ static int davinci_mcasp_hw_params(struct snd_pcm_substream *substream, int channels; struct snd_interval *pcm_channels = hw_param_interval(params, SNDRV_PCM_HW_PARAM_CHANNELS); + + /* If mcasp is BCLK master we need to set BCLK divider */ + if (mcasp->bclk_master) { + unsigned int bclk_freq = snd_soc_params_to_bclk(params); + if (mcasp->sysclk_freq % bclk_freq != 0) { + dev_err(mcasp->dev, "Can't produce requred BCLK\n"); + return -EINVAL; + } + davinci_mcasp_set_clkdiv( + cpu_dai, 1, mcasp->sysclk_freq / bclk_freq); + } + channels = pcm_channels->min;
active_serializers = (channels + slots - 1) / slots;
On Mon, Jan 27, 2014 at 05:37:52PM +0200, Jyri Sarha wrote:
Make BCLK divider setting implicite in hw_params call if McASP device is the bit clock master on the audio serial bus.
Applied, thanks.
Add machine driver support for BeagleBone-Black and other boards with tilcdc support and NXP TDA998X HDMI transmitter connected to McASP port in I2S mode. McASP produces the bit clock for the i2s bus from the masted clock by a simple divider and the available sample rates depend on the used master clock frequency. The only properly working sample format appears to be SNDRV_PCM_FORMAT_S32_LE. The other formats have been disabled. The 8 least significant bits of the 32 bit samples are ignored.
Signed-off-by: Jyri Sarha jsarha@ti.com cc: bcousson@baylibre.com --- .../bindings/sound/davinci-evm-audio.txt | 4 +- sound/soc/davinci/davinci-evm.c | 153 +++++++++++++++++++- 2 files changed, 152 insertions(+), 5 deletions(-)
diff --git a/Documentation/devicetree/bindings/sound/davinci-evm-audio.txt b/Documentation/devicetree/bindings/sound/davinci-evm-audio.txt index 963e100..2a535d9 100644 --- a/Documentation/devicetree/bindings/sound/davinci-evm-audio.txt +++ b/Documentation/devicetree/bindings/sound/davinci-evm-audio.txt @@ -1,7 +1,9 @@ * Texas Instruments SoC audio setups with TLV320AIC3X Codec
Required properties: -- compatible : "ti,da830-evm-audio" : forDM365/DA8xx/OMAPL1x/AM33xx +- compatible : + "ti,da830-evm-audio" : for DM365/DA8xx/OMAPL1x/AM33xx + "ti,am33xx-beaglebone-black-audio" : for Beaglebone-black HDMI audio - ti,model : The user-visible name of this sound complex. - ti,audio-codec : The phandle of the TLV320AIC3x audio codec - ti,mcasp-controller : The phandle of the McASP controller diff --git a/sound/soc/davinci/davinci-evm.c b/sound/soc/davinci/davinci-evm.c index d3e4cb0..00f1e83 100644 --- a/sound/soc/davinci/davinci-evm.c +++ b/sound/soc/davinci/davinci-evm.c @@ -21,6 +21,7 @@ #include <sound/core.h> #include <sound/pcm.h> #include <sound/soc.h> +#include <sound/pcm_params.h>
#include <asm/dma.h> #include <asm/mach-types.h> @@ -33,8 +34,13 @@ struct snd_soc_card_drvdata_davinci { struct clk *mclk; unsigned sysclk; + struct snd_pcm_hw_constraint_list *rate_constraint; };
+/* If changing sample format the tda998x configuration (REG_CTS_N) needs + to be changed. */ +#define TDA998X_SAMPLE_FORMAT SNDRV_PCM_FORMAT_S32_LE + static int evm_startup(struct snd_pcm_substream *substream) { struct snd_soc_pcm_runtime *rtd = substream->private_data; @@ -67,9 +73,10 @@ static int evm_hw_params(struct snd_pcm_substream *substream, struct snd_soc_dai *cpu_dai = rtd->cpu_dai; struct snd_soc_codec *codec = rtd->codec; struct snd_soc_card *soc_card = codec->card; + struct snd_soc_card_drvdata_davinci *drvdata = + snd_soc_card_get_drvdata(soc_card); + unsigned sysclk = drvdata->sysclk; int ret = 0; - unsigned sysclk = ((struct snd_soc_card_drvdata_davinci *) - snd_soc_card_get_drvdata(soc_card))->sysclk;
/* set the codec system clock */ ret = snd_soc_dai_set_sysclk(codec_dai, 0, sysclk, SND_SOC_CLOCK_OUT); @@ -84,12 +91,63 @@ static int evm_hw_params(struct snd_pcm_substream *substream, return 0; }
+static int evm_tda998x_startup(struct snd_pcm_substream *substream) +{ + struct snd_pcm_runtime *runtime = substream->runtime; + struct snd_soc_pcm_runtime *rtd = substream->private_data; + struct snd_soc_card *soc_card = rtd->codec->card; + struct snd_soc_card_drvdata_davinci *drvdata = + snd_soc_card_get_drvdata(soc_card); + struct snd_mask *fmt = constrs_mask(&runtime->hw_constraints, + SNDRV_PCM_HW_PARAM_FORMAT); + snd_mask_none(fmt); + snd_mask_set(fmt, TDA998X_SAMPLE_FORMAT); + + runtime->hw.rate_min = drvdata->rate_constraint->list[0]; + runtime->hw.rate_max = drvdata->rate_constraint->list[ + drvdata->rate_constraint->count - 1]; + runtime->hw.rates = SNDRV_PCM_RATE_KNOT; + + snd_pcm_hw_constraint_list(runtime, 0, SNDRV_PCM_HW_PARAM_RATE, + drvdata->rate_constraint); + snd_pcm_hw_constraint_minmax(runtime, SNDRV_PCM_HW_PARAM_CHANNELS, + 2, 2); + + return evm_startup(substream); +} + +static int evm_tda998x_hw_params(struct snd_pcm_substream *substream, + struct snd_pcm_hw_params *params) +{ + struct snd_soc_pcm_runtime *rtd = substream->private_data; + struct snd_soc_dai *cpu_dai = rtd->cpu_dai; + struct snd_soc_codec *codec = rtd->codec; + struct snd_soc_card *soc_card = codec->card; + struct platform_device *pdev = to_platform_device(soc_card->dev); + struct snd_soc_card_drvdata_davinci *drvdata = + snd_soc_card_get_drvdata(soc_card); + unsigned sysclk = drvdata->sysclk; + int ret; + + ret = snd_soc_dai_set_sysclk(cpu_dai, 0, sysclk, SND_SOC_CLOCK_IN); + if (ret < 0) + return ret; + + return ret; +} + static struct snd_soc_ops evm_ops = { .startup = evm_startup, .shutdown = evm_shutdown, .hw_params = evm_hw_params, };
+static struct snd_soc_ops evm_tda998x_ops = { + .startup = evm_tda998x_startup, + .shutdown = evm_shutdown, + .hw_params = evm_tda998x_hw_params, +}; + /* davinci-evm machine dapm widgets */ static const struct snd_soc_dapm_widget aic3x_dapm_widgets[] = { SND_SOC_DAPM_HP("Headphone Jack", NULL), @@ -156,6 +214,79 @@ static int evm_aic3x_init(struct snd_soc_pcm_runtime *rtd) return 0; }
+static unsigned int tda998x_hdmi_rates[] = { + 32000, + 44100, + 48000, + 88200, + 96000, +}; + +static struct snd_pcm_hw_constraint_list *evm_tda998x_rate_constraint( + struct snd_soc_card *soc_card) +{ + struct platform_device *pdev = to_platform_device(soc_card->dev); + struct snd_soc_card_drvdata_davinci *drvdata = + snd_soc_card_get_drvdata(soc_card); + unsigned sysclk = drvdata->sysclk; + struct snd_pcm_hw_constraint_list *ret; + unsigned int *rates; + int i, j = 0; + + ret = devm_kzalloc(soc_card->dev, sizeof(*ret), GFP_KERNEL); + rates = devm_kzalloc(soc_card->dev, sizeof(tda998x_hdmi_rates), + GFP_KERNEL); + if (!ret || !rates) + return NULL; + + ret->list = rates; + ret->mask = 0; + for (i = 0; i < ARRAY_SIZE(tda998x_hdmi_rates); i++) { + unsigned int bclk_freq = tda998x_hdmi_rates[i] * 2 * + snd_pcm_format_width(TDA998X_SAMPLE_FORMAT); + if (sysclk % bclk_freq == 0) { + rates[j++] = tda998x_hdmi_rates[i]; + dev_dbg(soc_card->dev, "Allowing rate %u\n", + tda998x_hdmi_rates[i]); + } + } + ret->count = j; + return ret; +} + +static const struct snd_soc_dapm_widget tda998x_dapm_widgets[] = { + SND_SOC_DAPM_OUTPUT("HDMI Out"), +}; + +static int evm_tda998x_init(struct snd_soc_pcm_runtime *rtd) +{ + struct snd_soc_dai *cpu_dai = rtd->cpu_dai; + struct snd_soc_dapm_context *dapm = &rtd->codec->dapm; + struct snd_soc_card *soc_card = rtd->codec->card; + struct snd_soc_card_drvdata_davinci *drvdata = + snd_soc_card_get_drvdata(soc_card); + int ret; + + ret = snd_soc_dai_set_clkdiv(cpu_dai, 0, 1); + if (ret < 0) + return ret; + + drvdata->rate_constraint = evm_tda998x_rate_constraint(soc_card); + + snd_soc_dapm_new_controls(dapm, tda998x_dapm_widgets, + ARRAY_SIZE(tda998x_dapm_widgets)); + + ret = snd_soc_of_parse_audio_routing(soc_card, "ti,audio-routing"); + + /* not connected */ + snd_soc_dapm_disable_pin(dapm, "RX"); + + /* always connected */ + snd_soc_dapm_enable_pin(dapm, "HDMI Out"); + + return 0; +} + /* davinci-evm digital audio interface glue - connects codec <--> CPU */ static struct snd_soc_dai_link dm6446_evm_dai = { .name = "TLV320AIC3X", @@ -341,7 +472,7 @@ static struct snd_soc_card da850_snd_soc_card = { #if defined(CONFIG_OF)
/* - * The struct is used as place holder. It will be completely + * The structs are used as place holders. They will be completely * filled with data from dt node. */ static struct snd_soc_dai_link evm_dai_tlv320aic3x = { @@ -354,10 +485,24 @@ static struct snd_soc_dai_link evm_dai_tlv320aic3x = { SND_SOC_DAIFMT_IB_NF, };
+static struct snd_soc_dai_link evm_dai_tda998x_hdmi = { + .name = "NXP TDA998x HDMI Chip", + .stream_name = "HDMI", + .codec_dai_name = "hdmi-hifi", + .ops = &evm_tda998x_ops, + .init = evm_tda998x_init, + .dai_fmt = (SND_SOC_DAIFMT_CBS_CFS | SND_SOC_DAIFMT_I2S | + SND_SOC_DAIFMT_IB_NF), +}; + static const struct of_device_id davinci_evm_dt_ids[] = { { .compatible = "ti,da830-evm-audio", - .data = (void *) &evm_dai_tlv320aic3x, + .data = &evm_dai_tlv320aic3x, + }, + { + .compatible = "ti,am33xx-beaglebone-black-audio", + .data = &evm_dai_tda998x_hdmi, }, { /* sentinel */ } };
On Mon, Jan 27, 2014 at 05:37:53PM +0200, Jyri Sarha wrote:
Add machine driver support for BeagleBone-Black and other boards with tilcdc support and NXP TDA998X HDMI transmitter connected to McASP port in I2S mode. McASP produces the bit clock for the i2s bus from the masted clock by a simple divider and the available sample rates
I have to say I agree with Lars' comments about where the code to set the constraints is here - I don't doubt that these limitations are valid but it would be better to factor them into the relevant chip drivers so that other systems with similar limitations can be handled correctly too.
On 01/27/2014 10:49 PM, Mark Brown wrote:
On Mon, Jan 27, 2014 at 05:37:53PM +0200, Jyri Sarha wrote:
Add machine driver support for BeagleBone-Black and other boards with tilcdc support and NXP TDA998X HDMI transmitter connected to McASP port in I2S mode. McASP produces the bit clock for the i2s bus from the masted clock by a simple divider and the available sample rates
I have to say I agree with Lars' comments about where the code to set the constraints is here - I don't doubt that these limitations are valid but it would be better to factor them into the relevant chip drivers so that other systems with similar limitations can be handled correctly too.
Ok, I'll push them into mcasp driver then.
Thanks, Jyri
Adds configuration option for HDMI audio support for AM33XX based boards with NXP TDA998x HDMI transmitter. The audio is connected to NXP TDA998x trough McASP running in i2s mode.
Signed-off-by: Jyri Sarha jsarha@ti.com --- sound/soc/davinci/Kconfig | 12 ++++++++++++ sound/soc/davinci/Makefile | 1 + 2 files changed, 13 insertions(+)
diff --git a/sound/soc/davinci/Kconfig b/sound/soc/davinci/Kconfig index a8ec1fc..40dd5d1 100644 --- a/sound/soc/davinci/Kconfig +++ b/sound/soc/davinci/Kconfig @@ -26,6 +26,18 @@ config SND_AM33XX_SOC_EVM AM335X-EVMSK, and BeagelBone with AudioCape boards have this setup.
+config SND_AM335X_SOC_NXPTDA_EVM + tristate "HDMI Audio for the AM33XX chip based boards with TDA998x" + depends on SND_DAVINCI_SOC && SOC_AM33XX + depends on DRM_TILCDC && DRM_I2C_NXP_TDA998X + select SND_SOC_HDMI_CODEC + select SND_DAVINCI_SOC_MCASP + help + Say Y or M if you want to add support for HDMI SoC audio on + AM33XX boards with NXP TDA998x HDMI transmitter. For example + BeagleBoneBack. The audio is connected to NXP TDA998x trough + McASP running in i2s mode. + config SND_DAVINCI_SOC_EVM tristate "SoC Audio support for DaVinci DM6446, DM355 or DM365 EVM" depends on SND_DAVINCI_SOC diff --git a/sound/soc/davinci/Makefile b/sound/soc/davinci/Makefile index 744d4d9..7587a70 100644 --- a/sound/soc/davinci/Makefile +++ b/sound/soc/davinci/Makefile @@ -13,3 +13,4 @@ obj-$(CONFIG_SND_DAVINCI_SOC_VCIF) += snd-soc-davinci-vcif.o snd-soc-evm-objs := davinci-evm.o
obj-$(CONFIG_SND_DAVINCI_SOC_GENERIC_EVM) += snd-soc-evm.o +obj-$(CONFIG_SND_AM335X_SOC_NXPTDA_EVM) += snd-soc-evm.o
The configuration is needed for HDMI audio. The "swap" and "mirr" parameters have to be correctly set in the configuration in order to have proper colors in the HDMI picture.
Signed-off-by: Jyri Sarha jsarha@ti.com cc: airlied@linux.ie --- drivers/gpu/drm/tilcdc/tilcdc_slave.c | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_slave.c b/drivers/gpu/drm/tilcdc/tilcdc_slave.c index 595068b..e43240a 100644 --- a/drivers/gpu/drm/tilcdc/tilcdc_slave.c +++ b/drivers/gpu/drm/tilcdc/tilcdc_slave.c @@ -19,6 +19,7 @@ #include <linux/pinctrl/pinmux.h> #include <linux/pinctrl/consumer.h> #include <drm/drm_encoder_slave.h> +#include <drm/i2c/tda998x.h>
#include "tilcdc_drv.h"
@@ -111,8 +112,29 @@ static const struct drm_encoder_helper_funcs slave_encoder_helper_funcs = { .restore = drm_i2c_encoder_restore, };
+static struct tda998x_encoder_params tda998x_pdata = { + .swap_b = 0x3, + .mirr_b = 0x0, + .swap_a = 0x2, + .mirr_a = 0x0, + .swap_d = 0x1, + .mirr_d = 0x0, + .swap_c = 0x0, + .mirr_c = 0x0, + .swap_f = 0x5, + .mirr_f = 0x0, + .swap_e = 0x4, + .mirr_e = 0x0, + .audio_cfg = 0x3, /* I2S mode */ + .audio_clk_cfg = 1, /* select clock pin */ + .audio_frame[1] = 1, /* channels - 1 */ + .audio_format = AFMT_I2S, + .audio_sample_rate = 48000, +}; + static const struct i2c_board_info info = { - I2C_BOARD_INFO("tda998x", 0x70) + I2C_BOARD_INFO("tda998x", 0x70), + .platform_data = &tda998x_pdata, };
static struct drm_encoder *slave_encoder_create(struct drm_device *dev,
This enables HDMI video support on Beaglebone-Black.
Signed-off-by: Jyri Sarha jsarha@ti.com cc: tony@atomide.com --- arch/arm/configs/omap2plus_defconfig | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/arch/arm/configs/omap2plus_defconfig b/arch/arm/configs/omap2plus_defconfig index bfa80a1..9288172 100644 --- a/arch/arm/configs/omap2plus_defconfig +++ b/arch/arm/configs/omap2plus_defconfig @@ -180,6 +180,9 @@ CONFIG_REGULATOR_TPS6507X=y CONFIG_REGULATOR_TPS65217=y CONFIG_REGULATOR_TPS65910=y CONFIG_REGULATOR_TWL4030=y +CONFIG_DRM=m +CONFIG_DRM_I2C_NXP_TDA998X=m +CONFIG_DRM_TILCDC=m CONFIG_FB=y CONFIG_FIRMWARE_EDID=y CONFIG_FB_MODE_HELPERS=y
Select following: CONFIG_SND_DAVINCI_SOC=m CONFIG_SND_AM335X_SOC_NXPTDA_EVM=m
Signed-off-by: Jyri Sarha jsarha@ti.com cc: tony@atomide.com --- arch/arm/configs/omap2plus_defconfig | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/arch/arm/configs/omap2plus_defconfig b/arch/arm/configs/omap2plus_defconfig index 9288172..62c4f51 100644 --- a/arch/arm/configs/omap2plus_defconfig +++ b/arch/arm/configs/omap2plus_defconfig @@ -214,6 +214,8 @@ CONFIG_SND_OMAP_SOC=m CONFIG_SND_OMAP_SOC_OMAP_TWL4030=m CONFIG_SND_OMAP_SOC_OMAP_ABE_TWL6040=m CONFIG_SND_OMAP_SOC_OMAP3_PANDORA=m +CONFIG_SND_DAVINCI_SOC=m +CONFIG_SND_AM335X_SOC_NXPTDA_EVM=m CONFIG_USB=y CONFIG_USB_DEBUG=y CONFIG_USB_ANNOUNCE_NEW_DEVICES=y
Hi,
I think you should try to get this in sync with the work Jean-Francois is currently doing[1]. Having the HDMI transmitter register a CODEC device is definitely the right approach, compared to the rather 'interesting' constraints stuff you do in patch 4.
- Lars
[1] http://lkml.org/lkml/2014/1/27/85
On 01/27/2014 04:37 PM, Jyri Sarha wrote:
Changes since RFC v2 version of the patches:
- Dropped out already applied: ASoC: hdmi-codec: Add devicetree binding with documentation
- Addresses Mark's comments here: http://mailman.alsa-project.org/pipermail/alsa-devel/2013-December/070605.ht...
- ASoC: davinci-evm: Add named clock reference to DT bindings
- Get rid of unnecessary castings
- Add mclk NULL checks
- Use devm_clk_get()
- Change clock name from "ti,codec-clock" to "mclk"
- Address Mark's comments here: http://mailman.alsa-project.org/pipermail/alsa-devel/2013-December/070606.ht... - ASoC: davinci-evm: HDMI audio support for TDA998x trough McASP I2S bus
- Get rid of unnecessary castings
- Update commit message
- Add: ASoC: davinci-mcasp: Set BCLK divider if McASP is BCLK master
- Use snd_soc_params_to_bclk()
Changes since the first RFC version of the patches:
- Drop out already applied: ASoC: hdmi-codec: Add SNDRV_PCM_FMTBIT_32_LE playback format
- Change sound node's compatible property form: "ti,am33xx-beaglebone-black" to "ti,am33xx-beaglebone-black-audio"
- Some minor style issue fixes from TI internal review
Jyri Sarha (8): clk: add gpio controlled clock ASoC: davinci-evm: Add named clock reference to DT bindings ASoC: davinci-mcasp: Set BCLK divider if McASP is BCLK master ASoC: davinci-evm: HDMI audio support for TDA998x trough McASP I2S bus ASoC: davinci: HDMI audio build for AM33XX and TDA998x drm/tilcdc: Add I2C HDMI audio config for tda998x ARM: OMAP2+: omap2plus_defconfig: Enable tilcdc and TDA998X HDMI support ARM: OMAP2+: omap2plus_defconfig: Enable BeagleBone Black HDMI audio support
.../devicetree/bindings/clock/gpio-clock.txt | 21 ++ .../bindings/sound/davinci-evm-audio.txt | 13 +- arch/arm/configs/omap2plus_defconfig | 5 + drivers/clk/Makefile | 1 + drivers/clk/clk-gpio.c | 210 +++++++++++++++++++ drivers/gpu/drm/tilcdc/tilcdc_slave.c | 24 ++- include/linux/clk-provider.h | 25 +++ sound/soc/davinci/Kconfig | 12 ++ sound/soc/davinci/Makefile | 1 + sound/soc/davinci/davinci-evm.c | 211 +++++++++++++++++++- sound/soc/davinci/davinci-mcasp.c | 20 ++ 11 files changed, 534 insertions(+), 9 deletions(-) create mode 100644 Documentation/devicetree/bindings/clock/gpio-clock.txt create mode 100644 drivers/clk/clk-gpio.c
On 01/27/2014 05:51 PM, Lars-Peter Clausen wrote:
Hi,
I think you should try to get this in sync with the work Jean-Francois is currently doing[1]. Having the HDMI transmitter register a CODEC device is definitely the right approach, compared to the rather 'interesting' constraints stuff you do in patch 4.
Oh, Jean-Francois's patches are now out. I'll take those into use, but I am afraid it still does not save me from the constraint stuff.
The most complex piece of code comes from limited sample-rate availability, which is coming Beaglebone-Black desing (available i2s master clock), not from the HDMI encoder itself. It does not help with the stereo only limitation either, because it comes from the one wire only audio data connection on BBB.
Support for S16_LE could maybe be added if the tda998x specific codec would fiddle with CTS_N predivider-setting (K select) according to the used sample width. But it appears Cobox plays all the sample formats fine without this change, so the question is if playing around with CTS_N predivider would break already working support there (something to be tested).
Best regards, Jyri
On 01/27/2014 05:17 PM, Jyri Sarha wrote:
On 01/27/2014 05:51 PM, Lars-Peter Clausen wrote:
Hi,
I think you should try to get this in sync with the work Jean-Francois is currently doing[1]. Having the HDMI transmitter register a CODEC device is definitely the right approach, compared to the rather 'interesting' constraints stuff you do in patch 4.
Oh, Jean-Francois's patches are now out. I'll take those into use, but I am afraid it still does not save me from the constraint stuff.
The most complex piece of code comes from limited sample-rate availability, which is coming Beaglebone-Black desing (available i2s master clock), not from the HDMI encoder itself. It does not help with the stereo only limitation either, because it comes from the one wire only audio data connection on BBB.
Support for S16_LE could maybe be added if the tda998x specific codec would fiddle with CTS_N predivider-setting (K select) according to the used sample width. But it appears Cobox plays all the sample formats fine without this change, so the question is if playing around with CTS_N predivider would break already working support there (something to be tested).
The ASoC core will set the constraints for the audio format/rate to the intersection of what is supported by the CODEC and the CPU DAI. So if the limitation comes from the CPU DAI the constraints should be applied there and not in the machine driver. Similar if the tda998x only supports 32 bit samples it should advertise this and the compound card will only support 32 bit samples.
- Lars
On 01/27/2014 06:32 PM, Lars-Peter Clausen wrote:
On 01/27/2014 05:17 PM, Jyri Sarha wrote:
On 01/27/2014 05:51 PM, Lars-Peter Clausen wrote:
Hi,
I think you should try to get this in sync with the work Jean-Francois is currently doing[1]. Having the HDMI transmitter register a CODEC device is definitely the right approach, compared to the rather 'interesting' constraints stuff you do in patch 4.
Oh, Jean-Francois's patches are now out. I'll take those into use, but I am afraid it still does not save me from the constraint stuff.
The most complex piece of code comes from limited sample-rate availability, which is coming Beaglebone-Black desing (available i2s master clock), not from the HDMI encoder itself. It does not help with the stereo only limitation either, because it comes from the one wire only audio data connection on BBB.
Support for S16_LE could maybe be added if the tda998x specific codec would fiddle with CTS_N predivider-setting (K select) according to the used sample width. But it appears Cobox plays all the sample formats fine without this change, so the question is if playing around with CTS_N predivider would break already working support there (something to be tested).
The ASoC core will set the constraints for the audio format/rate to the intersection of what is supported by the CODEC and the CPU DAI. So if the limitation comes from the CPU DAI the constraints should be applied there and not in the machine driver. Similar if the tda998x only supports 32 bit samples it should advertise this and the compound card will only support 32 bit samples.
Yes. I know. But these limitations come from the run time setup of the audio serial bus and those details are not available to the cpu dai at the register time.
If more of the McASP configuration data would be moved to DT, instead of giving it in set_sysclk, set_fmt, etc. calls it would be possible for the driver code produce more accurate constraints at register time. However that would change McASP driver a lot and would possibly break some of the legacy support.
Best regards, Jyri
On 01/27/2014 08:40 PM, Jyri Sarha wrote:
On 01/27/2014 06:32 PM, Lars-Peter Clausen wrote:
On 01/27/2014 05:17 PM, Jyri Sarha wrote:
On 01/27/2014 05:51 PM, Lars-Peter Clausen wrote:
Hi,
I think you should try to get this in sync with the work Jean-Francois is currently doing[1]. Having the HDMI transmitter register a CODEC device is definitely the right approach, compared to the rather 'interesting' constraints stuff you do in patch 4.
Oh, Jean-Francois's patches are now out. I'll take those into use, but I am afraid it still does not save me from the constraint stuff.
The most complex piece of code comes from limited sample-rate availability, which is coming Beaglebone-Black desing (available i2s master clock), not from the HDMI encoder itself. It does not help with the stereo only limitation either, because it comes from the one wire only audio data connection on BBB.
Support for S16_LE could maybe be added if the tda998x specific codec would fiddle with CTS_N predivider-setting (K select) according to the used sample width. But it appears Cobox plays all the sample formats fine without this change, so the question is if playing around with CTS_N predivider would break already working support there (something to be tested).
The ASoC core will set the constraints for the audio format/rate to the intersection of what is supported by the CODEC and the CPU DAI. So if the limitation comes from the CPU DAI the constraints should be applied there and not in the machine driver. Similar if the tda998x only supports 32 bit samples it should advertise this and the compound card will only support 32 bit samples.
Yes. I know. But these limitations come from the run time setup of the audio serial bus and those details are not available to the cpu dai at the register time.
If more of the McASP configuration data would be moved to DT, instead of giving it in set_sysclk, set_fmt, etc. calls it would be possible for the driver code produce more accurate constraints at register time. However that would change McASP driver a lot and would possibly break some of the legacy support.
There is nothing wrong with setting the constraints based on the parameters passed to set_sysclk or set_fmt, etc. In fact this is something that is often done by drivers.
- Lars
On Mon, 27 Jan 2014 18:17:54 +0200 Jyri Sarha jsarha@ti.com wrote:
Support for S16_LE could maybe be added if the tda998x specific codec would fiddle with CTS_N predivider-setting (K select) according to the used sample width. But it appears Cobox plays all the sample formats fine without this change, so the question is if playing around with CTS_N predivider would break already working support there (something to be tested).
Jyri,
Setting
cts_n = CTS_N_M(3) | CTS_N_K(1);
instead of K(3) in the I2S sequence of tda998x_configure_audio() in the tda998x driver works fine for me with S16_LE and S24_LE.
Does this solve your problem?
Russell, do you see any problem about this change?
On 01/27/2014 08:06 PM, Jean-Francois Moine wrote:
On Mon, 27 Jan 2014 18:17:54 +0200 Jyri Sarha jsarha@ti.com wrote:
Support for S16_LE could maybe be added if the tda998x specific codec would fiddle with CTS_N predivider-setting (K select) according to the used sample width. But it appears Cobox plays all the sample formats fine without this change, so the question is if playing around with CTS_N predivider would break already working support there (something to be tested).
Jyri,
Setting
cts_n = CTS_N_M(3) | CTS_N_K(1);
instead of K(3) in the I2S sequence of tda998x_configure_audio() in the tda998x driver works fine for me with S16_LE and S24_LE.
Does this solve your problem?
With that setting S16_LE works, but S32_LE does not. S24_LE does not really work at all on BBB, or it works but because the 8 most significant bits are zero the audio output is attenuated by 48 dB.
On the other hand S24_3LE does work, but then the problem is the clock rate support. On BBB the McASP IP is the bit clock master and provides the bit clock by simple divider from 24.576MHz oscillator. With 24 bit samples neither 44100 or 48000Hz sample rate can be supported (on the other hand 32kHz could be supported 24576000 % (24*2*32000) = 0).
The BBB HW price has squeezed so low that it can only implement the most basic use cases.
I would suggest to leave the CTS_N_K to the current setting (3), unless we can change the CTS_N_K on the fly according to the used sample format.
Best regards, Jyri
On Mon, 27 Jan 2014 21:31:59 +0200 Jyri Sarha jsarha@ti.com wrote:
I would suggest to leave the CTS_N_K to the current setting (3), unless we can change the CTS_N_K on the fly according to the used sample format.
Yes, this is possible:
- the tda998x codec may call the tda998x hdmi in the hw_params() function, i.e. when the sample format is known, and then,
- the tda998x_audio_update() function may have the audio parameters (struct snd_pcm_hw_params) as an argument, and CTS_N_K may be set to either 1, 2 or 3 for SNDRV_PCM_FORMAT_S16_LE / S24_LE and S32_LE.
This is working in my machine. Would it also work for you?
On 01/28/2014 11:23 AM, Jean-Francois Moine wrote:
On Mon, 27 Jan 2014 21:31:59 +0200 Jyri Sarha jsarha@ti.com wrote:
I would suggest to leave the CTS_N_K to the current setting (3), unless we can change the CTS_N_K on the fly according to the used sample format.
Yes, this is possible:
the tda998x codec may call the tda998x hdmi in the hw_params() function, i.e. when the sample format is known, and then,
the tda998x_audio_update() function may have the audio parameters (struct snd_pcm_hw_params) as an argument, and CTS_N_K may be set to either 1, 2 or 3 for SNDRV_PCM_FORMAT_S16_LE / S24_LE and S32_LE.
This is working in my machine. Would it also work for you?
I am having trouble getting the tda998x-codec working on BBB. The problem is I do not have a DT node for the tda998x driver. Instead I have tilcdc-slave node that provides pdata for the tda-driver.
I am thinking of solving the problem by adding a reference to the i2c-adapter hosting tda998x as an optional DT property to the codec node. I could then dig the driver instance from the i2c adapter's children. Any better ideas?
Best regards, Jyri
On Thu, 30 Jan 2014 14:20:56 +0200 Jyri Sarha jsarha@ti.com wrote:
I am having trouble getting the tda998x-codec working on BBB. The problem is I do not have a DT node for the tda998x driver. Instead I have tilcdc-slave node that provides pdata for the tda-driver.
I am thinking of solving the problem by adding a reference to the i2c-adapter hosting tda998x as an optional DT property to the codec node. I could then dig the driver instance from the i2c adapter's children. Any better ideas?
I better think about a 'normal' DT definition:
- in the DT, define the tda998x in a i2c subnode:
&i2c0 { tda998x: hdmi-encoder { compatible = "nxp,tda998x"; reg = <0x70>; /* the video ports are OK by default */ /* define the interrupt if you want to use it */ }; };
- in tilcdc_slave.c, in the function slave_encoder_create(), instead of using drm_i2c_encoder_init(), do quite the same, but without calling request_module() nor i2c_new_device().
This can be done as follows (the code is not compiled):
--------------------8<--------------------- static struct drm_encoder *slave_encoder_create(struct drm_device *dev, struct slave_module *mod) { struct slave_encoder *slave_encoder; struct drm_encoder *encoder; int ret; /* ------ added ------ */ struct device_node *np; static const struct of_device_id tda_dt[] = { { .compatible = "nxp,tda998x" }, { }, }; /* ------ end added ------ */
... no change ...
drm_encoder_helper_add(encoder, &slave_encoder_helper_funcs);
/* ------ added ------ */
/* search the tda998x device */ np = of_find_matching_node_and_match(NULL, tda_dt, NULL); if (np && of_device_is_available(np)) { struct i2c_client *i2c_client; struct drm_i2c_encoder_driver *encoder_drv; struct module *module;
/* the tda998x is in the DT */
i2c_client = of_find_i2c_device_by_node(np); of_node_put(np); if (!i2c_client) { dev_err(dev->dev, "no tda998x i2c client\n"); goto fail; }
to_encoder_slave(encoder)->bus_priv = i2c_client;
encoder_drv = to_drm_i2c_encoder_driver( to_i2c_driver(i2c_client->dev.driver));
/* lock the tda998x module in memory */ module = to_i2c_driver(i2c_client->dev.driver)->driver.owner; if (!module || !try_module_get(module)) { dev_err(dev->dev, "cannot get module %s\n", module->name); goto fail; }
ret = encoder_drv->encoder_init(i2c_client, dev, encoder_slave); if (ret < 0) { dev_err(dev->dev, "slave encoder init failed\n"); module_put(module); goto fail; } /* set_config is useless */ return encoder; }
/* ------ end added ------ */
ret = drm_i2c_encoder_init(dev, to_encoder_slave(encoder), mod->i2c, &info); if (ret) goto fail;
return encoder;
fail: slave_encoder_destroy(encoder); return NULL; } --------------------8<---------------------
When the tda998x is in the DT, the i2c_client is already created. It must not be freed, and so, the function drm_i2c_encoder_destroy() must not be called. But, the module must be explicitly unlocked in slave_encoder_destroy(), and then, there must be some flag in the structures for this job to be done...
participants (4)
-
Jean-Francois Moine -
Jyri Sarha -
Lars-Peter Clausen -
Mark Brown