Skip to contentSkip to conversation

Beaglebone-Black HDMI audio

RFC PATCH Applied Jyri Sarha 4 participants 23 messages Jan 27, 2014 – Jan 30, 2014

Subsystems (9)
lore
Share & tools
lore

Jyri Sarha

author
#0 lore raw

v3 cover 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.html
- 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.html - 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
Signature · 1 line
1.7.9.5
author
#1 lore raw

RFC PATCH v3 1/8 clk: add gpio controlled clock

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.

---
 .../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
Documentation/devicetree/bindings/clock/gpio-clock.txt new file +21 -0
new file mode 100644
index 0000000..54fea39
@@ -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>;
+	};
drivers/clk/Makefile +1 -0
index 7a10bc9..9616e3a 100644
@@ -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
drivers/clk/clk-gpio.c new file +210 -0

210 lines not on this page · raw message

include/linux/clk-provider.h +25 -0
index 7e59253..21082b2 100644
@@ -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
Signature · 1 line
1.7.9.5
author
#2 lore raw

RFC PATCH v3 2/8 ASoC: davinci-evm: Add named clock reference to DT bindings

In reply to Jyri Sarha

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.

---
 .../bindings/sound/davinci-evm-audio.txt           |    9 ++-
 sound/soc/davinci/davinci-evm.c                    |   58 +++++++++++++++++++-
 2 files changed, 64 insertions(+), 3 deletions(-)
Documentation/devicetree/bindings/sound/davinci-evm-audio.txt +8 -1
index 865178d..963e100 100644
@@ -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
sound/soc/davinci/davinci-evm.c +56 -2

96 lines not on this page · raw message

Signature · 1 line
1.7.9.5
author
#4 lore raw

RFC PATCH v3 3/8 ASoC: davinci-mcasp: Set BCLK divider if McASP is BCLK master

In reply to Jyri Sarha

Make BCLK divider setting implicite in hw_params call if McASP device is the bit clock master on the audio serial bus.

---
 sound/soc/davinci/davinci-mcasp.c |   20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)
sound/soc/davinci/davinci-mcasp.c +20 -0
index b7858bf..ae328be 100644
@@ -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;
Signature · 1 line
1.7.9.5
author
#6 lore raw

RFC PATCH v3 4/8 ASoC: davinci-evm: HDMI audio support for TDA998x trough McASP I2S bus

In reply to Jyri Sarha

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.

---
 .../bindings/sound/davinci-evm-audio.txt           |    4 +-
 sound/soc/davinci/davinci-evm.c                    |  153 +++++++++++++++++++-
 2 files changed, 152 insertions(+), 5 deletions(-)
Documentation/devicetree/bindings/sound/davinci-evm-audio.txt +3 -1
index 963e100..2a535d9 100644
@@ -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
sound/soc/davinci/davinci-evm.c +149 -4

207 lines not on this page · raw message

Signature · 1 line
1.7.9.5
author
#9 lore raw

RFC PATCH v3 5/8 ASoC: davinci: HDMI audio build for AM33XX and TDA998x

In reply to Jyri Sarha

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.

---
 sound/soc/davinci/Kconfig  |   12 ++++++++++++
 sound/soc/davinci/Makefile |    1 +
 2 files changed, 13 insertions(+)
sound/soc/davinci/Kconfig +12 -0
index a8ec1fc..40dd5d1 100644
@@ -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
sound/soc/davinci/Makefile +1 -0
index 744d4d9..7587a70 100644
@@ -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
Signature · 1 line
1.7.9.5
author
#10 lore raw

RFC PATCH v3 6/8 drm/tilcdc: Add I2C HDMI audio config for tda998x

In reply to Jyri Sarha

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.

---
 drivers/gpu/drm/tilcdc/tilcdc_slave.c |   24 +++++++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)
drivers/gpu/drm/tilcdc/tilcdc_slave.c +23 -1
index 595068b..e43240a 100644
@@ -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,
Signature · 1 line
1.7.9.5
author
#11 lore raw

RFC PATCH v3 7/8 ARM: OMAP2+: omap2plus_defconfig: Enable tilcdc and TDA998X HDMI support

In reply to Jyri Sarha

This enables HDMI video support on Beaglebone-Black.

---
 arch/arm/configs/omap2plus_defconfig |    3 +++
 1 file changed, 3 insertions(+)
arch/arm/configs/omap2plus_defconfig +3 -0
index bfa80a1..9288172 100644
@@ -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
Signature · 1 line
1.7.9.5
author
#12 lore raw

RFC PATCH v3 8/8 ARM: OMAP2+: omap2plus_defconfig: Enable BeagleBone Black HDMI audio support

In reply to Jyri Sarha

Select following:
CONFIG_SND_DAVINCI_SOC=m
CONFIG_SND_AM335X_SOC_NXPTDA_EVM=m

---
 arch/arm/configs/omap2plus_defconfig |    2 ++
 1 file changed, 2 insertions(+)
arch/arm/configs/omap2plus_defconfig +2 -0
index 9288172..62c4f51 100644
@@ -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
Signature · 1 line
1.7.9.5
#13 lore raw
In reply to Jyri Sarha

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

Quoted 52 lines · Jyri Sarha

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.html
- 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.html - 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
author
#14 lore raw
Quoted 7 lines · Lars-Peter Clausen

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

#15 lore raw
Quoted 18 lines · Jyri Sarha
Quoted 7 lines · Lars-Peter Clausen

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.

Quoted 5 lines

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

#18 lore raw
In reply to Jyri Sarha
Quoted 6 lines · On Mon, 27 Jan 2014 18:17:54 +0200 Jyri Sarha <[email protected]> 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?

Signature · 6 lines
Ken ar c'hentañ	|	      ** Breizh ha Linux atav! **
Jef		|		http://moinejf.free.fr/
_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel
maintainer Applied
#3 lore raw

Re: [PATCH RFC v3 2/8] ASoC: davinci-evm: Add named clock reference to DT bindings

In reply to Jyri Sarha
Quoted 3 lines · Jyri Sarha

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.

author
#19 lore raw
In reply to Jean-Francois Moine
Quoted 12 lines · Jean-Francois Moine
Quoted 6 lines · On Mon, 27 Jan 2014 18:17:54 +0200 Jyri Sarha <[email protected]> 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,

Quoted 9 lines

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

author
#16 lore raw
In reply to Lars-Peter Clausen
Quoted 25 lines · Lars-Peter Clausen
Quoted 18 lines · Jyri Sarha
Quoted 7 lines · Lars-Peter Clausen

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.

Quoted 5 lines

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).

Quoted 7 lines

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

#17 lore raw
Quoted 34 lines · Jyri Sarha
Quoted 25 lines · Lars-Peter Clausen
Quoted 18 lines · Jyri Sarha
Quoted 7 lines · Lars-Peter Clausen

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.

Quoted 5 lines

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).

Quoted 7 lines

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.

Quoted 8 lines

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

maintainer Applied
#5 lore raw

Re: [PATCH RFC v3 3/8] ASoC: davinci-mcasp: Set BCLK divider if McASP is BCLK master

In reply to Jyri Sarha
Quoted 2 lines · Jyri Sarha

Make BCLK divider setting implicite in hw_params call if McASP device is the bit clock master on the audio serial bus.

Applied, thanks.

maintainer
#7 lore raw

Re: [PATCH RFC v3 4/8] ASoC: davinci-evm: HDMI audio support for TDA998x trough McASP I2S bus

In reply to Jyri Sarha
Quoted 4 lines · Jyri Sarha

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.

author
#8 lore raw

Re: [PATCH RFC v3 4/8] ASoC: davinci-evm: HDMI audio support for TDA998x trough McASP I2S bus

Quoted 6 lines · Mark Brown
Quoted 4 lines · Jyri Sarha

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

Quoted 6 lines

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

#20 lore raw
In reply to Jyri Sarha
Quoted 2 lines · On Mon, 27 Jan 2014 21:31:59 +0200 Jyri Sarha <[email protected]> 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?

Signature · 6 lines
Ken ar c'hentañ	|	      ** Breizh ha Linux atav! **
Jef		|		http://moinejf.free.fr/
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
author
#21 lore raw
Quoted 8 lines · Jean-Francois Moine
Quoted 2 lines · On Mon, 27 Jan 2014 21:31:59 +0200 Jyri Sarha <[email protected]> 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:

Quoted 9 lines

- 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

#22 lore raw
Quoted 8 lines · On Thu, 30 Jan 2014 14:20:56 +0200 Jyri Sarha <[email protected]> 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...

Signature · 6 lines
Ken ar c'hentañ	|	      ** Breizh ha Linux atav! **
Jef		|		http://moinejf.free.fr/
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html