[alsa-devel] [PATCH v14 0/3] ASoC: tda998x: add a codec to the HDMI transmitter
This patch series asks for Russell King's patch
http://article.gmane.org/gmane.linux.ports.arm.kernel/411574 "drm/edid: add function to help find SADs"
to be applied.
v14: (Mark Brown) - protect the CODEC from edid change - change some structure names - remove the static pointer to the HDMI driver functions - get the ELD from a pointer in the device private area - prevent audio from streaming many sources at the same time - build the CODEC DAI's from two different samples - use EXPORT_SYMBOL_GPL for exported functions - simplify exchanges between audio and video by using common data in the device private area v13: - rebase on 4.2.0-rc3 - remove Russell's patches v12: - use Russell King's DRM ELD helper (Mark Brown) v11: - reduce the patch series to adding the tda998x codec only v10: - add the generic dt-card - define the audio ports from a DT graph of ports (Russell King) - reuse HDMI constants (Andrew Jackson - Jyri Sarha) - alloc rate_constraints in codec (Jyri Sarha) - fix bad number of channels (Jyri Sarha) - correct codec generation from config (Russell King - Jyri Sarha) - no module init/exit (Russell King) v9: - back to a TDA998x specific CODEC - more comments - change magic values to constants v8: - change some comments about the patches v7: - remove the change of the K predivider (Jyri Sarha) - add S24_3LE and S32_LE as possible audio formats (Jyri Sarha) - don't move the struct priv2 definition and use the slave encoder private data as the device private data (Russell King) - remove the useless request_module (Russell King/Mark Brown) - don't lock the HDMI module (Russell King) - use platform_device_unregister to remove the codec (Russell King) v6: - extend the HDMI CODEC instead of using a specific CODEC v5: - use the TDA998x private data instead of a specific area for the CODEC interface - the CODEC is TDA998x specific (Mark Brown) v4: - remove all the TDA998x specific stuff from the CODEC - move the EDID scan from the CODEC to the TDA998x - move the CODEC to sound/soc (Mark Brown) - update the audio_sample_rate from the EDID (Andrew Jackson) v3: fix bad rate (Andrew Jackson) v2: check double stream start (Mark Brown)
Jean-Francois Moine (3): drm/i2c: tda998x: Add support of a DT graph of ports drm/i2c: tda998x: Change drvdata for audio extension ASoC: tda998x: add a codec to the HDMI transmitter
.../devicetree/bindings/drm/i2c/tda998x.txt | 51 +++++++ drivers/gpu/drm/i2c/tda998x_drv.c | 163 +++++++++++++++++++-- include/sound/tda998x.h | 18 +++ sound/soc/codecs/Kconfig | 6 + sound/soc/codecs/Makefile | 2 + sound/soc/codecs/tda998x.c | 141 ++++++++++++++++++ 6 files changed, 370 insertions(+), 11 deletions(-) create mode 100644 include/sound/tda998x.h create mode 100644 sound/soc/codecs/tda998x.c
The tda998x CODEC maintains the audio constraints according to the HDMI device parameters (EDID) and sets dynamically the input ports in the TDA998x I2C driver on start/stop audio streaming.
Signed-off-by: Jean-Francois Moine moinejf@free.fr --- drivers/gpu/drm/i2c/tda998x_drv.c | 71 ++++++++++++++++++- include/sound/tda998x.h | 10 +++ sound/soc/codecs/Kconfig | 6 ++ sound/soc/codecs/Makefile | 2 + sound/soc/codecs/tda998x.c | 141 ++++++++++++++++++++++++++++++++++++++ 5 files changed, 227 insertions(+), 3 deletions(-) create mode 100644 sound/soc/codecs/tda998x.c
diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c index 3504ef9..a271a53 100644 --- a/drivers/gpu/drm/i2c/tda998x_drv.c +++ b/drivers/gpu/drm/i2c/tda998x_drv.c @@ -32,6 +32,7 @@ #define DBG(fmt, ...) DRM_DEBUG(fmt"\n", ##__VA_ARGS__)
struct tda998x_priv { + struct tda998x_audio audio; /* audio and video common field */ struct i2c_client *cec; struct i2c_client *hdmi; struct mutex mutex; @@ -48,8 +49,6 @@ struct tda998x_priv { wait_queue_head_t wq_edid; volatile int wq_edid_wait; struct drm_encoder *encoder; - - struct tda998x_audio audio; };
#define to_tda998x_priv(x) ((struct tda998x_priv *)to_encoder_slave(x)->slave_priv) @@ -552,6 +551,9 @@ tda998x_reset(struct tda998x_priv *priv)
/* Write the default value MUX register */ reg_write(priv, REG_MUX_VP_VIP_OUT, 0x24); + + /* disable audio input */ + reg_write(priv, REG_ENA_AP, 0); }
/* handle HDMI connect/disconnect */ @@ -758,6 +760,42 @@ tda998x_configure_audio(struct tda998x_priv *priv, tda998x_write_aif(priv, p); }
+#if IS_ENABLED(CONFIG_SND_SOC_TDA998X) +/* tda998x audio codec interface */ + +/* switch the audio port and initialize the audio parameters for streaming */ +static int tda998x_set_audio_input(struct device *dev, + int port_index, + unsigned sample_rate) +{ + struct tda998x_priv *priv = dev_get_drvdata(dev); + struct tda998x_encoder_params *p = &priv->params; + + if (!priv->encoder->crtc) + return -ENODEV; + + /* if no port, just disable the audio port */ + if (port_index == PORT_NONE) { + reg_write(priv, REG_ENA_AP, 0); + return 0; + } + + /* if same audio parameters, just enable the audio port */ + if (p->audio_cfg == priv->audio.ports[port_index] && + p->audio_sample_rate == sample_rate) { + reg_write(priv, REG_ENA_AP, p->audio_cfg); + return 0; + } + + p->audio_format = priv->audio.port_types[port_index]; + p->audio_clk_cfg = p->audio_format == AFMT_SPDIF ? 0 : 1; + p->audio_cfg = priv->audio.ports[port_index]; + p->audio_sample_rate = sample_rate; + tda998x_configure_audio(priv, &priv->encoder->crtc->hwmode, p); + return 0; +} +#endif /* SND_SOC */ + /* DRM encoder functions */
static void tda998x_encoder_set_config(struct tda998x_priv *priv, @@ -777,8 +815,10 @@ static void tda998x_encoder_set_config(struct tda998x_priv *priv, (p->mirr_f ? VIP_CNTRL_2_MIRR_F : 0);
priv->params = *p; +#if IS_ENABLED(CONFIG_SND_SOC_TDA998X) priv->audio.port_types[0] = p->audio_format; priv->audio.ports[0] = p->audio_cfg; +#endif }
static void tda998x_encoder_dpms(struct tda998x_priv *priv, int mode) @@ -1029,9 +1069,11 @@ tda998x_encoder_mode_set(struct tda998x_priv *priv,
tda998x_write_avi(priv, adjusted_mode);
+#if !IS_ENABLED(CONFIG_SND_SOC_TDA998X) if (priv->params.audio_cfg) tda998x_configure_audio(priv, adjusted_mode, &priv->params); +#endif } }
@@ -1107,6 +1149,9 @@ tda998x_encoder_get_modes(struct tda998x_priv *priv, struct edid *edid; int n;
+#if IS_ENABLED(CONFIG_SND_SOC_TDA998X) + priv->audio.eld = NULL; +#endif if (priv->rev == TDA19988) reg_clear(priv, REG_TX4, TX4_PD_RAM);
@@ -1123,6 +1168,14 @@ tda998x_encoder_get_modes(struct tda998x_priv *priv, drm_mode_connector_update_edid_property(connector, edid); n = drm_add_edid_modes(connector, edid); priv->is_hdmi_sink = drm_detect_hdmi_monitor(edid); + +#if IS_ENABLED(CONFIG_SND_SOC_TDA998X) + if (priv->is_hdmi_sink) { + drm_edid_to_eld(connector, edid); + priv->audio.eld = connector->eld; + } +#endif + kfree(edid);
return n; @@ -1158,6 +1211,10 @@ static void tda998x_destroy(struct tda998x_priv *priv) cancel_delayed_work_sync(&priv->dwork); }
+#if IS_ENABLED(CONFIG_SND_SOC_TDA998X) + if (priv->audio.ports[0]) + tda9998x_codec_unregister(&priv->hdmi->dev); +#endif i2c_unregister_device(priv->cec); }
@@ -1294,6 +1351,9 @@ static int tda998x_create(struct i2c_client *client, struct tda998x_priv *priv) priv->vip_cntrl_1 = VIP_CNTRL_1_SWAP_C(0) | VIP_CNTRL_1_SWAP_D(1); priv->vip_cntrl_2 = VIP_CNTRL_2_SWAP_E(4) | VIP_CNTRL_2_SWAP_F(5);
+ priv->params.audio_frame[1] = 1; /* channels - 1 */ + priv->params.audio_sample_rate = 48000; /* 48kHz */ + priv->current_page = 0xff; priv->hdmi = client; /* CEC I2C address bound to TDA998x I2C addr by configuration pins */ @@ -1399,14 +1459,19 @@ static int tda998x_create(struct i2c_client *client, struct tda998x_priv *priv) if (ret < 0) goto fail;
- /* initialize the default audio configuration */ + /* initialize the audio configuration */ if (priv->audio.ports[0]) { +#if IS_ENABLED(CONFIG_SND_SOC_TDA998X) + priv->audio.set_audio_input = tda998x_set_audio_input; + tda9998x_codec_register(&client->dev); +#else priv->params.audio_cfg = priv->audio.ports[0]; priv->params.audio_format = priv->audio.port_types[0]; priv->params.audio_clk_cfg = priv->params.audio_format == AFMT_SPDIF ? 0 : 1; +#endif } } else {
diff --git a/include/sound/tda998x.h b/include/sound/tda998x.h index bef1da7..91a069c 100644 --- a/include/sound/tda998x.h +++ b/include/sound/tda998x.h @@ -1,8 +1,18 @@ #ifndef SND_TDA998X_H #define SND_TDA998X_H
+/* port index for audio stream stop */ +#define PORT_NONE (-1) + struct tda998x_audio { u8 ports[2]; /* AP value */ u8 port_types[2]; /* AFMT_xxx */ + u8 *eld; + int (*set_audio_input)(struct device *dev, + int port_index, + unsigned sample_rate); }; + +int tda9998x_codec_register(struct device *dev); +void tda9998x_codec_unregister(struct device *dev); #endif diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig index efaafce..08b73bd 100644 --- a/sound/soc/codecs/Kconfig +++ b/sound/soc/codecs/Kconfig @@ -105,6 +105,7 @@ config SND_SOC_ALL_CODECS select SND_SOC_TAS2552 if I2C select SND_SOC_TAS5086 if I2C select SND_SOC_TAS571X if I2C + select SND_SOC_TDA998X if DRM_I2C_NXP_TDA998X select SND_SOC_TFA9879 if I2C select SND_SOC_TLV320AIC23_I2C if I2C select SND_SOC_TLV320AIC23_SPI if SPI_MASTER @@ -622,6 +623,11 @@ config SND_SOC_TAS571X tristate "Texas Instruments TAS5711/TAS5717/TAS5719 power amplifiers" depends on I2C
+config SND_SOC_TDA998X + def_tristate y + select SND_PCM_ELD + depends on DRM_I2C_NXP_TDA998X + config SND_SOC_TFA9879 tristate "NXP Semiconductors TFA9879 amplifier" depends on I2C diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile index cf160d9..819d689 100644 --- a/sound/soc/codecs/Makefile +++ b/sound/soc/codecs/Makefile @@ -108,6 +108,7 @@ snd-soc-sta529-objs := sta529.o snd-soc-stac9766-objs := stac9766.o snd-soc-tas5086-objs := tas5086.o snd-soc-tas571x-objs := tas571x.o +snd-soc-tda998x-objs := tda998x.o snd-soc-tfa9879-objs := tfa9879.o snd-soc-tlv320aic23-objs := tlv320aic23.o snd-soc-tlv320aic23-i2c-objs := tlv320aic23-i2c.o @@ -292,6 +293,7 @@ obj-$(CONFIG_SND_SOC_STAC9766) += snd-soc-stac9766.o obj-$(CONFIG_SND_SOC_TAS2552) += snd-soc-tas2552.o obj-$(CONFIG_SND_SOC_TAS5086) += snd-soc-tas5086.o obj-$(CONFIG_SND_SOC_TAS571X) += snd-soc-tas571x.o +obj-$(CONFIG_SND_SOC_TDA998X) += snd-soc-tda998x.o obj-$(CONFIG_SND_SOC_TFA9879) += snd-soc-tfa9879.o obj-$(CONFIG_SND_SOC_TLV320AIC23) += snd-soc-tlv320aic23.o obj-$(CONFIG_SND_SOC_TLV320AIC23_I2C) += snd-soc-tlv320aic23-i2c.o diff --git a/sound/soc/codecs/tda998x.c b/sound/soc/codecs/tda998x.c new file mode 100644 index 0000000..0153b80 --- /dev/null +++ b/sound/soc/codecs/tda998x.c @@ -0,0 +1,141 @@ +/* + * ALSA SoC TDA998x CODEC + * + * Copyright (C) 2015 Jean-Francois Moine + * + * 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. + */ + +#include <linux/module.h> +#include <sound/soc.h> +#include <linux/of.h> +#include <linux/of_device.h> +#include <sound/pcm_drm_eld.h> +#include <drm/i2c/tda998x.h> +#include <sound/tda998x.h> + +static int tda998x_codec_startup(struct snd_pcm_substream *substream, + struct snd_soc_dai *dai) +{ + struct snd_pcm_runtime *runtime = substream->runtime; + struct tda998x_audio *tda998x_audio = dev_get_drvdata(dai->dev); + u8 *eld; + + eld = tda998x_audio->eld; + if (!eld) + return -ENODEV; + return snd_pcm_hw_constraint_eld(runtime, eld); +} + +/* ask the HDMI transmitter to activate the audio input port */ +static int tda998x_codec_hw_params(struct snd_pcm_substream *substream, + struct snd_pcm_hw_params *params, + struct snd_soc_dai *dai) +{ + struct tda998x_audio *tda998x_audio = dev_get_drvdata(dai->dev); + + return tda998x_audio->set_audio_input(dai->dev, dai->id, + params_rate(params)); +} + +static void tda998x_codec_shutdown(struct snd_pcm_substream *substream, + struct snd_soc_dai *dai) +{ + struct tda998x_audio *tda998x_audio = dev_get_drvdata(dai->dev); + + tda998x_audio->set_audio_input(dai->dev, PORT_NONE, 0); +} + +static const struct snd_soc_dai_ops tda998x_codec_ops = { + .startup = tda998x_codec_startup, + .hw_params = tda998x_codec_hw_params, + .shutdown = tda998x_codec_shutdown, +}; + +#define TDA998X_FORMATS (SNDRV_PCM_FMTBIT_S16_LE | \ + SNDRV_PCM_FMTBIT_S20_3LE | \ + SNDRV_PCM_FMTBIT_S24_LE | \ + SNDRV_PCM_FMTBIT_S32_LE) + +static const struct snd_soc_dai_driver tda998x_dai_i2s = { + .name = "i2s-hifi", + .playback = { + .stream_name = "HDMI I2S Playback", + .channels_min = 1, + .channels_max = 8, + .rates = SNDRV_PCM_RATE_CONTINUOUS, + .rate_min = 5512, + .rate_max = 192000, + .formats = TDA998X_FORMATS, + }, + .ops = &tda998x_codec_ops, +}; +static const struct snd_soc_dai_driver tda998x_dai_spdif = { + .name = "spdif-hifi", + .playback = { + .stream_name = "HDMI SPDIF Playback", + .channels_min = 1, + .channels_max = 2, + .rates = SNDRV_PCM_RATE_CONTINUOUS, + .rate_min = 22050, + .rate_max = 192000, + .formats = TDA998X_FORMATS, + }, + .ops = &tda998x_codec_ops, +}; + +static const struct snd_soc_dapm_widget tda998x_widgets[] = { + SND_SOC_DAPM_OUTPUT("hdmi-out"), +}; +static const struct snd_soc_dapm_route tda998x_routes[] = { + { "hdmi-out", NULL, "HDMI I2S Playback" }, + { "hdmi-out", NULL, "HDMI SPDIF Playback" }, +}; + +static struct snd_soc_codec_driver tda998x_codec_drv = { + .dapm_widgets = tda998x_widgets, + .num_dapm_widgets = ARRAY_SIZE(tda998x_widgets), + .dapm_routes = tda998x_routes, + .num_dapm_routes = ARRAY_SIZE(tda998x_routes), + .ignore_pmdown_time = true, +}; + +int tda9998x_codec_register(struct device *dev) +{ + struct snd_soc_dai_driver *dais, *p_dai; + struct tda998x_audio *tda998x_audio = dev_get_drvdata(dev); + int i, ndais; + + /* build the DAIs */ + for (ndais = 0; ndais < ARRAY_SIZE(tda998x_audio->ports); ndais++) { + if (!tda998x_audio->ports[ndais]) + break; + } + dais = devm_kzalloc(dev, sizeof(*dais) * ndais, GFP_KERNEL); + if (!dais) + return -ENOMEM; + for (i = 0, p_dai = dais; i < ndais ; i++, p_dai++) { + if (tda998x_audio->port_types[i] == AFMT_I2S) + memcpy(p_dai, &tda998x_dai_i2s, sizeof(*p_dai)); + else + memcpy(p_dai, &tda998x_dai_spdif, sizeof(*p_dai)); + p_dai->id = i; + } + + return snd_soc_register_codec(dev, + &tda998x_codec_drv, + dais, ndais); +} +EXPORT_SYMBOL_GPL(tda9998x_codec_register); + +void tda9998x_codec_unregister(struct device *dev) +{ + snd_soc_unregister_codec(dev); +} +EXPORT_SYMBOL_GPL(tda9998x_codec_unregister); + +MODULE_AUTHOR("Jean-Francois Moine moinejf@free.fr"); +MODULE_DESCRIPTION("TDA998X CODEC"); +MODULE_LICENSE("GPL");
Two kinds of ports may be declared in a DT graph of ports: video and audio. This patch accepts the port value from a video port as an alternative to the video-ports property. It also accepts audio ports in the case the transmitter is not used as a slave encoder. The new file include/sound/tda998x.h prepares to the definition of a tda998x CODEC.
Signed-off-by: Jean-Francois Moine moinejf@free.fr --- .../devicetree/bindings/drm/i2c/tda998x.txt | 51 ++++++++++++ drivers/gpu/drm/i2c/tda998x_drv.c | 90 +++++++++++++++++++--- include/sound/tda998x.h | 8 ++ 3 files changed, 140 insertions(+), 9 deletions(-) create mode 100644 include/sound/tda998x.h
diff --git a/Documentation/devicetree/bindings/drm/i2c/tda998x.txt b/Documentation/devicetree/bindings/drm/i2c/tda998x.txt index e9e4bce..35f6a80 100644 --- a/Documentation/devicetree/bindings/drm/i2c/tda998x.txt +++ b/Documentation/devicetree/bindings/drm/i2c/tda998x.txt @@ -16,6 +16,35 @@ Optional properties:
- video-ports: 24 bits value which defines how the video controller output is wired to the TDA998x input - default: <0x230145> + This property is not used when ports are defined. + +Optional nodes: + + - port: up to three ports. + The ports are defined according to [1]. + + Video port. + There may be only one video port. + This one must contain the following property: + + - port-type: must be "rgb" + + and may contain the optional property: + + - reg: 24 bits value which defines how the video controller + output is wired to the TDA998x input (video pins) + When absent, the default value is <0x230145>. + + Audio ports. + There may be one or two audio ports. + These ones must contain the following properties: + + - port-type: must be "i2s" or "spdif" + + - reg: 8 bits value which defines how the audio controller + output is wired to the TDA998x input (audio pins) + +[1] Documentation/devicetree/bindings/graph.txt
Example:
@@ -26,4 +55,26 @@ Example: interrupts = <27 2>; /* falling edge */ pinctrl-0 = <&pmx_camera>; pinctrl-names = "default"; + + port@230145 { + port-type = "rgb"; + reg = <0x230145>; + hdmi_0: endpoint { + remote-endpoint = <&lcd0_0>; + }; + }; + port@3 { /* AP1 = I2S */ + port-type = "i2s"; + reg = <0x03>; + tda998x_i2s: endpoint { + remote-endpoint = <&audio1_i2s>; + }; + }; + port@4 { /* AP2 = S/PDIF */ + port-type = "spdif"; + reg = <0x04>; + tda998x_spdif: endpoint { + remote-endpoint = <&audio1_spdif1>; + }; + }; }; diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c index 424228b..0952eac 100644 --- a/drivers/gpu/drm/i2c/tda998x_drv.c +++ b/drivers/gpu/drm/i2c/tda998x_drv.c @@ -27,6 +27,7 @@ #include <drm/drm_edid.h> #include <drm/drm_of.h> #include <drm/i2c/tda998x.h> +#include <sound/tda998x.h>
#define DBG(fmt, ...) DRM_DEBUG(fmt"\n", ##__VA_ARGS__)
@@ -47,6 +48,8 @@ struct tda998x_priv { wait_queue_head_t wq_edid; volatile int wq_edid_wait; struct drm_encoder *encoder; + + struct tda998x_audio audio; };
#define to_tda998x_priv(x) ((struct tda998x_priv *)to_encoder_slave(x)->slave_priv) @@ -774,6 +777,8 @@ static void tda998x_encoder_set_config(struct tda998x_priv *priv, (p->mirr_f ? VIP_CNTRL_2_MIRR_F : 0);
priv->params = *p; + priv->audio.port_types[0] = p->audio_format; + priv->audio.ports[0] = p->audio_cfg; }
static void tda998x_encoder_dpms(struct tda998x_priv *priv, int mode) @@ -1230,9 +1235,57 @@ static struct drm_encoder_slave_funcs tda998x_encoder_slave_funcs = {
/* I2C driver functions */
+static int tda998x_parse_ports(struct tda998x_priv *priv, + struct device_node *np) +{ + struct device_node *of_port; + const char *port_type; + int ret, audio_index, reg, afmt; + + audio_index = 0; + for_each_child_of_node(np, of_port) { + if (!of_port->name + || of_node_cmp(of_port->name, "port") != 0) + continue; + ret = of_property_read_string(of_port, "port-type", + &port_type); + if (ret < 0) + continue; + ret = of_property_read_u32(of_port, "reg", ®); + if (strcmp(port_type, "rgb") == 0) { + if (!ret) { /* video reg is optional */ + priv->vip_cntrl_0 = reg >> 16; + priv->vip_cntrl_1 = reg >> 8; + priv->vip_cntrl_2 = reg; + } + continue; + } + if (strcmp(port_type, "i2s") == 0) + afmt = AFMT_I2S; + else if (strcmp(port_type, "spdif") == 0) + afmt = AFMT_SPDIF; + else + continue; + if (ret < 0) { + dev_err(&priv->hdmi->dev, "missing reg for %s\n", + port_type); + return ret; + } + if (audio_index >= ARRAY_SIZE(priv->audio.ports)) { + dev_err(&priv->hdmi->dev, "too many audio ports\n"); + break; + } + priv->audio.ports[audio_index] = reg; + priv->audio.port_types[audio_index] = afmt; + audio_index++; + } + return 0; +} + static int tda998x_create(struct i2c_client *client, struct tda998x_priv *priv) { struct device_node *np = client->dev.of_node; + struct device_node *of_port; u32 video; int rev_lo, rev_hi, ret; unsigned short cec_addr; @@ -1337,15 +1390,34 @@ static int tda998x_create(struct i2c_client *client, struct tda998x_priv *priv) /* enable EDID read irq: */ reg_set(priv, REG_INT_FLAGS_2, INT_FLAGS_2_EDID_BLK_RD);
- if (!np) - return 0; /* non-DT */ - - /* get the optional video properties */ - ret = of_property_read_u32(np, "video-ports", &video); - if (ret == 0) { - priv->vip_cntrl_0 = video >> 16; - priv->vip_cntrl_1 = video >> 8; - priv->vip_cntrl_2 = video; + /* get the device tree parameters */ + if (np) { + of_port = of_get_child_by_name(np, "port"); + if (of_port) { /* graph of ports */ + of_node_put(of_port); + ret = tda998x_parse_ports(priv, np); + if (ret < 0) + goto fail; + + /* initialize the default audio configuration */ + if (priv->audio.ports[0]) { + priv->params.audio_cfg = priv->audio.ports[0]; + priv->params.audio_format = + priv->audio.port_types[0]; + priv->params.audio_clk_cfg = + priv->params.audio_format == + AFMT_SPDIF ? 0 : 1; + } + } else { + + /* optional video properties */ + ret = of_property_read_u32(np, "video-ports", &video); + if (ret == 0) { + priv->vip_cntrl_0 = video >> 16; + priv->vip_cntrl_1 = video >> 8; + priv->vip_cntrl_2 = video; + } + } }
return 0; diff --git a/include/sound/tda998x.h b/include/sound/tda998x.h new file mode 100644 index 0000000..bef1da7 --- /dev/null +++ b/include/sound/tda998x.h @@ -0,0 +1,8 @@ +#ifndef SND_TDA998X_H +#define SND_TDA998X_H + +struct tda998x_audio { + u8 ports[2]; /* AP value */ + u8 port_types[2]; /* AFMT_xxx */ +}; +#endif
On 05/08/15 11:18, Jean-Francois Moine wrote:
Two kinds of ports may be declared in a DT graph of ports: video and audio. This patch accepts the port value from a video port as an alternative to the video-ports property. It also accepts audio ports in the case the transmitter is not used as a slave encoder. The new file include/sound/tda998x.h prepares to the definition of a tda998x CODEC.
Signed-off-by: Jean-Francois Moine moinejf@free.fr
.../devicetree/bindings/drm/i2c/tda998x.txt | 51 ++++++++++++ drivers/gpu/drm/i2c/tda998x_drv.c | 90 +++++++++++++++++++--- include/sound/tda998x.h | 8 ++ 3 files changed, 140 insertions(+), 9 deletions(-) create mode 100644 include/sound/tda998x.h
diff --git a/Documentation/devicetree/bindings/drm/i2c/tda998x.txt b/Documentation/devicetree/bindings/drm/i2c/tda998x.txt index e9e4bce..35f6a80 100644 --- a/Documentation/devicetree/bindings/drm/i2c/tda998x.txt +++ b/Documentation/devicetree/bindings/drm/i2c/tda998x.txt @@ -16,6 +16,35 @@ Optional properties:
- video-ports: 24 bits value which defines how the video controlleroutput is wired to the TDA998x input - default: <0x230145>
- This property is not used when ports are defined.
+Optional nodes:
- port: up to three ports.
- The ports are defined according to [1].
- Video port.
- There may be only one video port.
- This one must contain the following property:
- port-type: must be "rgb"
- and may contain the optional property:
- reg: 24 bits value which defines how the video controller
output is wired to the TDA998x input (video pins)When absent, the default value is <0x230145>.
Using reg property for something else than for address of some kind seems confusing to me. Should we just add an explicit property rgb mapping?
- Audio ports.
- There may be one or two audio ports.
- These ones must contain the following properties:
- port-type: must be "i2s" or "spdif"
- reg: 8 bits value which defines how the audio controller
output is wired to the TDA998x input (audio pins)
Here I do not even understand what what the values 3 ad 4 stand for. Also when trying to make a device file following the above binding I get errors related the different widths of for the register property values (I do not have the exact error at hand right now), but that prevented me from using these patches when I last tried them.
Anyway having some clearly defined property that explicitly defines the audio pins would make more sense to me. Even if that is not possible due lack of proper documentation it would be better not add to the confusion by unusual usage of reg property.
Best regards, Jyri
ps. Did you ever give my generic hdmi codec patch a try?
+[1] Documentation/devicetree/bindings/graph.txt
Example:
@@ -26,4 +55,26 @@ Example: interrupts = <27 2>; /* falling edge */ pinctrl-0 = <&pmx_camera>; pinctrl-names = "default";
port@230145 {port-type = "rgb";reg = <0x230145>;hdmi_0: endpoint {remote-endpoint = <&lcd0_0>;};};port@3 { /* AP1 = I2S */port-type = "i2s";reg = <0x03>; explicittda998x_i2s: endpoint {remote-endpoint = <&audio1_i2s>;};};port@4 { /* AP2 = S/PDIF */port-type = "spdif";reg = <0x04>;tda998x_spdif: endpoint {remote-endpoint = <&audio1_spdif1>;};};diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c index 424228b..0952eac 100644 --- a/drivers/gpu/drm/i2c/tda998x_drv.c +++ b/drivers/gpu/drm/i2c/tda998x_drv.c @@ -27,6 +27,7 @@ #include <drm/drm_edid.h> #include <drm/drm_of.h> #include <drm/i2c/tda998x.h> +#include <sound/tda998x.h>
#define DBG(fmt, ...) DRM_DEBUG(fmt"\n", ##__VA_ARGS__)
@@ -47,6 +48,8 @@ struct tda998x_priv { wait_queue_head_t wq_edid; volatile int wq_edid_wait; struct drm_encoder *encoder;
struct tda998x_audio audio; };
#define to_tda998x_priv(x) ((struct tda998x_priv *)to_encoder_slave(x)->slave_priv)
@@ -774,6 +777,8 @@ static void tda998x_encoder_set_config(struct tda998x_priv *priv, (p->mirr_f ? VIP_CNTRL_2_MIRR_F : 0);
priv->params = *p;
priv->audio.port_types[0] = p->audio_format;
priv->audio.ports[0] = p->audio_cfg; } explicit
static void tda998x_encoder_dpms(struct tda998x_priv *priv, int mode)
@@ -1230,9 +1235,57 @@ static struct drm_encoder_slave_funcs tda998x_encoder_slave_funcs = {
/* I2C driver functions */
+static int tda998x_parse_ports(struct tda998x_priv *priv,
struct device_node *np)+{
- struct device_node *of_port;
- const char *port_type;
- int ret, audio_index, reg, afmt;
- audio_index = 0;
- for_each_child_of_node(np, of_port) {
if (!of_port->name|| of_node_cmp(of_port->name, "port") != 0)continue;ret = of_property_read_string(of_port, "port-type",&port_type);if (ret < 0)continue;ret = of_property_read_u32(of_port, "reg", ®);if (strcmp(port_type, "rgb") == 0) {if (!ret) { /* video reg is optional */priv->vip_cntrl_0 = reg >> 16;priv->vip_cntrl_1 = reg >> 8;priv->vip_cntrl_2 = reg;}continue;}if (strcmp(port_type, "i2s") == 0)afmt = AFMT_I2S;else if (strcmp(port_type, "spdif") == 0)afmt = AFMT_SPDIF;elsecontinue;if (ret < 0) {dev_err(&priv->hdmi->dev, "missing reg for %s\n",port_type);return ret;}if (audio_index >= ARRAY_SIZE(priv->audio.ports)) {dev_err(&priv->hdmi->dev, "too many audio ports\n");break;}priv->audio.ports[audio_index] = reg;priv->audio.port_types[audio_index] = afmt;audio_index++;- }
- return 0;
+}
- static int tda998x_create(struct i2c_client *client, struct tda998x_priv *priv) { struct device_node *np = client->dev.of_node;
- struct device_node *of_port; u32 video; int rev_lo, rev_hi, ret; unsigned short cec_addr;
@@ -1337,15 +1390,34 @@ static int tda998x_create(struct i2c_client *client, struct tda998x_priv *priv) /* enable EDID read irq: */ reg_set(priv, REG_INT_FLAGS_2, INT_FLAGS_2_EDID_BLK_RD);
- if (!np)
return 0; /* non-DT */- /* get the optional video properties */
- ret = of_property_read_u32(np, "video-ports", &video);
- if (ret == 0) {
priv->vip_cntrl_0 = video >> 16;priv->vip_cntrl_1 = video >> 8;priv->vip_cntrl_2 = video;
/* get the device tree parameters */
if (np) {
of_port = of_get_child_by_name(np, "port");if (of_port) { /* graph of ports */of_node_put(of_port);ret = tda998x_parse_ports(priv, np);if (ret < 0)goto fail;/* initialize the default audio configuration */if (priv->audio.ports[0]) {priv->params.audio_cfg = priv->audio.ports[0];priv->params.audio_format =priv->audio.port_types[0];priv->params.audio_clk_cfg =priv->params.audio_format == explicitAFMT_SPDIF ? 0 : 1;}} else {/* optional video properties */ret = of_property_read_u32(np, "video-ports", &video);if (ret == 0) {priv->vip_cntrl_0 = video >> 16;priv->vip_cntrl_1 = video >> 8;priv->vip_cntrl_2 = video;}}}
return 0;
diff --git a/include/sound/tda998x.h b/include/sound/tda998x.h new file mode 100644 index 0000000..bef1da7 --- /dev/null +++ b/include/sound/tda998x.h @@ -0,0 +1,8 @@ +#ifndef SND_TDA998X_H +#define SND_TDA998X_H
+struct tda998x_audio {
- u8 ports[2]; /* AP value */
- u8 port_types[2]; /* AFMT_xxx */
+}; +#endif
On Mon, 3 Aug 2015 17:56:17 +0300 Jyri Sarha jsarha@ti.com wrote:
On 05/08/15 11:18, Jean-Francois Moine wrote:
Two kinds of ports may be declared in a DT graph of ports: video and audio. This patch accepts the port value from a video port as an alternative to the video-ports property. It also accepts audio ports in the case the transmitter is not used as a slave encoder. The new file include/sound/tda998x.h prepares to the definition of a tda998x CODEC.
Signed-off-by: Jean-Francois Moine moinejf@free.fr
.../devicetree/bindings/drm/i2c/tda998x.txt | 51 ++++++++++++ drivers/gpu/drm/i2c/tda998x_drv.c | 90 +++++++++++++++++++--- include/sound/tda998x.h | 8 ++ 3 files changed, 140 insertions(+), 9 deletions(-) create mode 100644 include/sound/tda998x.h
diff --git a/Documentation/devicetree/bindings/drm/i2c/tda998x.txt b/Documentation/devicetree/bindings/drm/i2c/tda998x.txt index e9e4bce..35f6a80 100644 --- a/Documentation/devicetree/bindings/drm/i2c/tda998x.txt +++ b/Documentation/devicetree/bindings/drm/i2c/tda998x.txt @@ -16,6 +16,35 @@ Optional properties:
- video-ports: 24 bits value which defines how the video controlleroutput is wired to the TDA998x input - default: <0x230145>
- This property is not used when ports are defined.
+Optional nodes:
- port: up to three ports.
- The ports are defined according to [1].
- Video port.
- There may be only one video port.
- This one must contain the following property:
- port-type: must be "rgb"
- and may contain the optional property:
- reg: 24 bits value which defines how the video controller
output is wired to the TDA998x input (video pins)When absent, the default value is <0x230145>.Using reg property for something else than for address of some kind seems confusing to me. Should we just add an explicit property rgb mapping?
Indeed, there could be an attribute as 'port-value'.
- Audio ports.
- There may be one or two audio ports.
- These ones must contain the following properties:
- port-type: must be "i2s" or "spdif"
- reg: 8 bits value which defines how the audio controller
output is wired to the TDA998x input (audio pins)Here I do not even understand what what the values 3 ad 4 stand for.
These values come from the TDA19988 documentation (the TDA9988 and TDA9989 have the same video and audio input registers). - 0x03 is WS (Word Select - bit 0) = 1 (I2S) and AP (Audio Pin) = 1 (bit 1) - 0x04 is WS = 0 (S/PDIF) and AP = 2 (bit 2)
Also when trying to make a device file following the above binding I get errors related the different widths of for the register property values (I do not have the exact error at hand right now), but that prevented me from using these patches when I last tried them.
Strange. I have no error.
Anyway having some clearly defined property that explicitly defines the audio pins would make more sense to me. Even if that is not possible due lack of proper documentation it would be better not add to the confusion by unusual usage of reg property.
I don't remember from where I got the TDA19988 documentation, but, anyway, the port values are the same as the ones found in the first kernel provided by the Cubox manufacturer.
Best regards, Jyri
ps. Did you ever give my generic hdmi codec patch a try?
No, because I don't need a so complex codec. Mine has no codec device, no private data, no clock, and it is less than 150 lines of code. Also, as my machine is now obsolete, I will stop any development for it and keep the kernel I have and which works fine enough for me.
Best regards.
On Mon, Aug 03, 2015 at 05:56:17PM +0300, Jyri Sarha wrote:
On 05/08/15 11:18, Jean-Francois Moine wrote:
Two kinds of ports may be declared in a DT graph of ports: video and audio. This patch accepts the port value from a video port as an alternative to the video-ports property. It also accepts audio ports in the case the transmitter is not used as a slave encoder. The new file include/sound/tda998x.h prepares to the definition of a tda998x CODEC.
Signed-off-by: Jean-Francois Moine moinejf@free.fr
.../devicetree/bindings/drm/i2c/tda998x.txt | 51 ++++++++++++ drivers/gpu/drm/i2c/tda998x_drv.c | 90 +++++++++++++++++++--- include/sound/tda998x.h | 8 ++ 3 files changed, 140 insertions(+), 9 deletions(-) create mode 100644 include/sound/tda998x.h
diff --git a/Documentation/devicetree/bindings/drm/i2c/tda998x.txt b/Documentation/devicetree/bindings/drm/i2c/tda998x.txt index e9e4bce..35f6a80 100644 --- a/Documentation/devicetree/bindings/drm/i2c/tda998x.txt +++ b/Documentation/devicetree/bindings/drm/i2c/tda998x.txt @@ -16,6 +16,35 @@ Optional properties:
- video-ports: 24 bits value which defines how the video controller
output is wired to the TDA998x input - default: <0x230145>
- This property is not used when ports are defined.
+Optional nodes:
- port: up to three ports.
- The ports are defined according to [1].
- Video port.
- There may be only one video port.
- This one must contain the following property:
- port-type: must be "rgb"
- and may contain the optional property:
- reg: 24 bits value which defines how the video controller
output is wired to the TDA998x input (video pins)When absent, the default value is <0x230145>.Using reg property for something else than for address of some kind seems confusing to me. Should we just add an explicit property rgb mapping?
It's what ePAPR requires.
The problem is when you have multiple nodes called the same thing, you need to add the @number suffix to the name, and if you do that, ePAPR says you must also have a reg property, where the reg property matches the number given after the @.
See https://www.power.org/wp-content/uploads/2012/06/Power_ePAPR_APPROVED_v1.1.p... page 15, 2.2.1 "Node names".
Here I do not even understand what what the values 3 ad 4 stand for.
It's the audio input pin number (which is a sort-of bus address.)
Anyway having some clearly defined property that explicitly defines the audio pins would make more sense to me. Even if that is not possible due lack of proper documentation it would be better not add to the confusion by unusual usage of reg property.
As ePAPR requires the reg= property with the @unit-address part, this is the most sane approach.
The device drvdata is used for component bind, but points to the encoder/connector structure which is hidden from the slave encoder. For audio extension, the slave encoder private data must be accessible, so, this patch changes drvdata to the slave encoder private data and sets it in case of slave encoder use.
Signed-off-by: Jean-Francois Moine moinejf@free.fr --- drivers/gpu/drm/i2c/tda998x_drv.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c index 0952eac..3504ef9 100644 --- a/drivers/gpu/drm/i2c/tda998x_drv.c +++ b/drivers/gpu/drm/i2c/tda998x_drv.c @@ -1453,6 +1453,8 @@ static int tda998x_encoder_init(struct i2c_client *client, encoder_slave->slave_priv = priv; encoder_slave->slave_funcs = &tda998x_encoder_slave_funcs;
+ dev_set_drvdata(&client->dev, priv); + return 0; }
@@ -1580,7 +1582,7 @@ static int tda998x_bind(struct device *dev, struct device *master, void *data) if (!priv) return -ENOMEM;
- dev_set_drvdata(dev, priv); + dev_set_drvdata(dev, &priv->base);
if (dev->of_node) crtcs = drm_of_find_possible_crtcs(drm, dev->of_node); @@ -1639,7 +1641,9 @@ err_encoder: static void tda998x_unbind(struct device *dev, struct device *master, void *data) { - struct tda998x_priv2 *priv = dev_get_drvdata(dev); + struct tda998x_priv *priv_s = dev_get_drvdata(dev); + struct tda998x_priv2 *priv = + container_of(priv_s, struct tda998x_priv2, base);
drm_connector_cleanup(&priv->connector); drm_encoder_cleanup(&priv->encoder);
participants (3)
-
Jean-Francois Moine -
Jyri Sarha -
Russell King - ARM Linux