Re: [PATCH v9 3/4] ASoC: tda998x: add a codec to the HDMI transmitter

[Andrew Jackson <Andrew.Jackson@arm.com>]

On 01/07/15 10:51, Jean-Francois Moine wrote:
> This patch adds a CODEC to the NXP TDA998x HDMI transmitter.
> 
> The CODEC handles both I2S and S/PDIF inputs.
> It maintains the audio format and rate 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/Kconfig       |   1 +
>  drivers/gpu/drm/i2c/tda998x_drv.c | 139 ++++++++++++++++++++++++++++--
>  include/sound/tda998x.h           |  23 +++++
>  sound/soc/codecs/Kconfig          |   4 +
>  sound/soc/codecs/Makefile         |   2 +
>  sound/soc/codecs/tda998x.c        | 175 ++++++++++++++++++++++++++++++++++++++
>  6 files changed, 339 insertions(+), 5 deletions(-)
>  create mode 100644 include/sound/tda998x.h
>  create mode 100644 sound/soc/codecs/tda998x.c
> 
> diff --git a/drivers/gpu/drm/i2c/Kconfig b/drivers/gpu/drm/i2c/Kconfig
> index 22c7ed6..24fc975 100644
> --- a/drivers/gpu/drm/i2c/Kconfig
> +++ b/drivers/gpu/drm/i2c/Kconfig
> @@ -28,6 +28,7 @@ config DRM_I2C_SIL164
>  config DRM_I2C_NXP_TDA998X
>         tristate "NXP Semiconductors TDA998X HDMI encoder"
>         default m if DRM_TILCDC
> +       select SND_SOC_TDA998X

Do you need this since sound/soc/codecs/Kconfig conditionally brings in the
CODEC driver?

>         help
>           Support for NXP Semiconductors TDA998X HDMI encoders.
> 
> diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
> index ce1478d..a26a516 100644
> --- a/drivers/gpu/drm/i2c/tda998x_drv.c
> +++ b/drivers/gpu/drm/i2c/tda998x_drv.c
> @@ -27,6 +27,11 @@
>  #include <drm/drm_encoder_slave.h>
>  #include <drm/drm_edid.h>
>  #include <drm/i2c/tda998x.h>
> +#include <sound/tda998x.h>
> +
> +#if defined(CONFIG_SND_SOC_TDA998X) || defined(CONFIG_SND_SOC_TDA998X_MODULE)
> +#define WITH_CODEC 1
> +#endif
> 
>  #define DBG(fmt, ...) DRM_DEBUG(fmt"\n", ##__VA_ARGS__)
> 
> @@ -47,6 +52,10 @@ struct tda998x_priv {
>         struct drm_encoder *encoder;
> 
>         u8 audio_ports[2];
> +#ifdef WITH_CODEC
> +       u8 sad[3];                      /* Short Audio Descriptor */
> +       struct snd_pcm_hw_constraint_list rate_constraints;
> +#endif
>  };
> 
>  #define to_tda998x_priv(x)  ((struct tda998x_priv *)to_encoder_slave(x)->slave_priv)
> @@ -730,11 +739,109 @@ tda998x_configure_audio(struct tda998x_priv *priv,
>         tda998x_write_aif(priv, p);
>  }
> 
> +#ifdef WITH_CODEC
> +/* tda998x audio codec interface */
> +
> +/* return the audio parameters extracted from the last EDID */
> +static int tda998x_get_audio_var(struct device *dev,
> +                               u8 **sad,
> +                       struct snd_pcm_hw_constraint_list **rate_constraints)
> +{
> +       struct tda998x_priv *priv = dev_get_drvdata(dev);
> +
> +       if (!priv->encoder->crtc
> +        || !priv->is_hdmi_sink)
> +               return -ENODEV;
> +
> +       *sad = priv->sad;
> +       *rate_constraints = &priv->rate_constraints;
> +       return 0;
> +}
> +
> +/* 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,
> +                               int sample_format)
> +{
> +       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 = port_index == PORT_SPDIF ? AFMT_SPDIF : AFMT_I2S;
> +       p->audio_clk_cfg = port_index == PORT_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;
> +}
> +
> +/* get the audio capabilities from the EDID */
> +static void tda998x_get_audio_caps(struct tda998x_priv *priv,
> +                               struct drm_connector *connector)
> +{
> +       u8 *eld = connector->eld;
> +       u8 *sad;
> +       int sad_count;
> +       unsigned eld_ver, mnl;
> +       u8 max_channels, rate_mask, fmt;
> +
> +       /* adjust the hw params from the ELD (EDID) */
> +       eld_ver = eld[0] >> 3;
> +       if (eld_ver != 2 && eld_ver != 31)
> +               return;
> +
> +       mnl = eld[4] & 0x1f;
> +       if (mnl > 16)
> +               return;
> +
> +       sad_count = eld[5] >> 4;
> +       sad = eld + 20 + mnl;
> +
> +       /* Start from the basic audio settings */
> +       max_channels = 1;
> +       rate_mask = 0;
> +       fmt = 0;
> +       while (sad_count--) {
> +               switch (sad[0] & 0x78) {
> +               case 0x08:                      /* SAD uncompressed audio */
> +                       if ((sad[0] & 7) > max_channels)
> +                               max_channels = sad[0] & 7;
> +                       rate_mask |= sad[1];
> +                       fmt |= sad[2] & 0x07;
> +                       break;
> +               }
> +               sad += 3;
> +       }
> +
> +       priv->sad[0] = max_channels;
> +       priv->sad[1] = rate_mask;
> +       priv->sad[2] = fmt;
> +}
> +#endif /* WITH_CODEC */
> +
>  /* DRM encoder functions */
> 
>  static void tda998x_encoder_set_config(struct tda998x_priv *priv,
>                                        const struct tda998x_encoder_params *p)
>  {
> +       int port_index;
> +
>         priv->vip_cntrl_0 = VIP_CNTRL_0_SWAP_A(p->swap_a) |
>                             (p->mirr_a ? VIP_CNTRL_0_MIRR_A : 0) |
>                             VIP_CNTRL_0_SWAP_B(p->swap_b) |
> @@ -749,6 +856,8 @@ static void tda998x_encoder_set_config(struct tda998x_priv *priv,
>                             (p->mirr_f ? VIP_CNTRL_2_MIRR_F : 0);
> 
>         priv->params = *p;
> +       port_index = p->audio_format == AFMT_SPDIF ? PORT_SPDIF : PORT_I2S;
> +       priv->audio_ports[port_index] = p->audio_cfg;
>  }
> 
>  static void tda998x_encoder_dpms(struct tda998x_priv *priv, int mode)
> @@ -1142,6 +1251,15 @@ 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);
> +
> +#ifdef WITH_CODEC
> +               /* set the audio parameters from the EDID */
> +               if (priv->is_hdmi_sink) {
> +                       drm_edid_to_eld(connector, edid);
> +                       tda998x_get_audio_caps(priv, connector);
> +               }
> +#endif
> +
>                 kfree(edid);
>         }
> 
> @@ -1176,6 +1294,10 @@ static void tda998x_destroy(struct tda998x_priv *priv)
>         if (priv->hdmi->irq)
>                 free_irq(priv->hdmi->irq, priv);
> 
> +#ifdef WITH_CODEC
> +       tda9998x_codec_unregister(&priv->hdmi->dev);
> +#endif
> +
>         i2c_unregister_device(priv->cec);
>  }
> 
> @@ -1384,26 +1506,33 @@ static int tda998x_create(struct i2c_client *client, struct tda998x_priv *priv)
>                                 break;
>                         }
>                         if (strcmp(p, "spdif") == 0) {
> -                               priv->audio_ports[0] = port;
> +                               priv->audio_ports[PORT_SPDIF] = port;
>                         } else if (strcmp(p, "i2s") == 0) {
> -                               priv->audio_ports[1] = port;
> +                               priv->audio_ports[PORT_I2S] = port;
>                         } else {
>                                 dev_err(&client->dev,
>                                         "bad audio-port-names '%s'\n", p);
>                                 break;
>                         }
>                 }
> -               if (priv->audio_ports[0]) {
> -                       priv->params.audio_cfg = priv->audio_ports[0];
> +               if (priv->audio_ports[PORT_SPDIF]) {
> +                       priv->params.audio_cfg = priv->audio_ports[PORT_SPDIF];
>                         priv->params.audio_format = AFMT_SPDIF;
>                         priv->params.audio_clk_cfg = 0;
>                 } else {
> -                       priv->params.audio_cfg = priv->audio_ports[1];
> +                       priv->params.audio_cfg = priv->audio_ports[PORT_I2S];
>                         priv->params.audio_format = AFMT_I2S;
>                         priv->params.audio_clk_cfg = 1;
>                 }
>         }
> 
> +#ifdef WITH_CODEC
> +       /* create the audio CODEC */
> +       if (priv->audio_ports[PORT_SPDIF] || priv->audio_ports[PORT_I2S])
> +               tda9998x_codec_register(&client->dev,
> +                                       tda998x_get_audio_var,
> +                                       tda998x_set_audio_input);
> +#endif
>         return 0;
> 
>  fail:
> diff --git a/include/sound/tda998x.h b/include/sound/tda998x.h
> new file mode 100644
> index 0000000..97cff30
> --- /dev/null
> +++ b/include/sound/tda998x.h
> @@ -0,0 +1,23 @@
> +#ifndef SND_TDA998X_H
> +#define SND_TDA998X_H
> +
> +#include <sound/pcm.h>
> +
> +/* port indexes */
> +#define PORT_NONE (-1)
> +#define PORT_SPDIF 0
> +#define PORT_I2S 1
> +
> +typedef int (*get_audio_var_t)(struct device *dev,
> +                               u8 **sad,
> +                       struct snd_pcm_hw_constraint_list **rate_constraints);
> +typedef int (*set_audio_input_t)(struct device *dev,
> +                               int port_index,
> +                               unsigned sample_rate,
> +                               int sample_format);
> +
> +int tda9998x_codec_register(struct device *dev,
> +                       get_audio_var_t get_audio_var_i,
> +                       set_audio_input_t set_audio_input_i);
> +void tda9998x_codec_unregister(struct device *dev);
> +#endif
> diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
> index 8349f98..456c549 100644
> --- a/sound/soc/codecs/Kconfig
> +++ b/sound/soc/codecs/Kconfig
> @@ -102,6 +102,7 @@ config SND_SOC_ALL_CODECS
>         select SND_SOC_STAC9766 if SND_SOC_AC97_BUS
>         select SND_SOC_TAS2552 if I2C
>         select SND_SOC_TAS5086 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
> @@ -600,6 +601,9 @@ config SND_SOC_TAS5086
>         tristate "Texas Instruments TAS5086 speaker amplifier"
>         depends on I2C
> 
> +config SND_SOC_TDA998X
> +       tristate
> +
>  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 bbdfd1e..44b4fda 100644
> --- a/sound/soc/codecs/Makefile
> +++ b/sound/soc/codecs/Makefile
> @@ -104,6 +104,7 @@ snd-soc-sta350-objs := sta350.o
>  snd-soc-sta529-objs := sta529.o
>  snd-soc-stac9766-objs := stac9766.o
>  snd-soc-tas5086-objs := tas5086.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
> @@ -282,6 +283,7 @@ obj-$(CONFIG_SND_SOC_STA529)   += snd-soc-sta529.o
>  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_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..b055570
> --- /dev/null
> +++ b/sound/soc/codecs/tda998x.c
> @@ -0,0 +1,175 @@
> +/*
> + * 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_params.h>
> +#include <sound/tda998x.h>
> +
> +/* functions in tda998x_drv */
> +static get_audio_var_t get_audio_var;
> +static set_audio_input_t set_audio_input;
> +
> +static int tda998x_codec_startup(struct snd_pcm_substream *substream,
> +                                struct snd_soc_dai *dai)
> +{
> +       struct snd_pcm_runtime *runtime = substream->runtime;
> +       struct snd_pcm_hw_constraint_list *rate_constraints;
> +       u8 *sad;                /* Short Audio Descriptor (3 bytes) */
> +#define SAD_MX_CHAN_I 0                        /* max number of chennels - 1 */
> +#define SAD_RATE_MASK_I 1              /* rate mask */
> +#define SAD_FMT_I 2                    /* formats */
> +#define  SAD_FMT_16 0x01
> +#define  SAD_FMT_20 0x02
> +#define  SAD_FMT_24 0x04

Wouldn't these defines be better in sound/tda998x.h, with an appropriate prefix, then
you could use them in gpu/drm/i2c/tda998x_drv.c.

	Andrew

> +       u64 formats;
> +       int ret;
> +       static const u32 hdmi_rates[] = {
> +               32000, 44100, 48000, 88200, 96000, 176400, 192000
> +       };
> +
> +       /* get the EDID values and the rate constraints buffer */
> +       ret = get_audio_var(dai->dev, &sad, &rate_constraints);
> +       if (ret < 0)
> +               return ret;                     /* no screen */
> +
> +       /* convert the EDID values to audio constraints */
> +       rate_constraints->list = hdmi_rates;
> +       rate_constraints->count = ARRAY_SIZE(hdmi_rates);
> +       rate_constraints->mask = sad[SAD_RATE_MASK_I];
> +       snd_pcm_hw_constraint_list(runtime, 0,
> +                                  SNDRV_PCM_HW_PARAM_RATE,
> +                                  rate_constraints);
> +
> +       formats = 0;
> +       if (sad[SAD_FMT_I] & SAD_FMT_16)
> +               formats |= SNDRV_PCM_FMTBIT_S16_LE;
> +       if (sad[SAD_FMT_I] & SAD_FMT_20)
> +               formats |= SNDRV_PCM_FMTBIT_S20_3LE;
> +       if (sad[SAD_FMT_I] & SAD_FMT_24)
> +               formats |= SNDRV_PCM_FMTBIT_S24_LE |
> +                          SNDRV_PCM_FMTBIT_S24_3LE |
> +                          SNDRV_PCM_FMTBIT_S32_LE;
> +       snd_pcm_hw_constraint_mask64(runtime,
> +                               SNDRV_PCM_HW_PARAM_FORMAT,
> +                               formats);
> +
> +       snd_pcm_hw_constraint_minmax(runtime,
> +                               SNDRV_PCM_HW_PARAM_CHANNELS,
> +                               1, sad[SAD_MX_CHAN_I]);
> +       return 0;
> +}
> +
> +static int tda998x_codec_hw_params(struct snd_pcm_substream *substream,
> +                                  struct snd_pcm_hw_params *params,
> +                                  struct snd_soc_dai *dai)
> +{
> +       return set_audio_input(dai->dev, dai->id,
> +                               params_rate(params),
> +                               params_format(params));
> +}
> +
> +static void tda998x_codec_shutdown(struct snd_pcm_substream *substream,
> +                                  struct snd_soc_dai *dai)
> +{
> +       set_audio_input(dai->dev, PORT_NONE, 0, 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 struct snd_soc_dai_driver tda998x_dais[] = {
> +       {
> +               .name = "spdif-hifi",
> +               .id = PORT_SPDIF,
> +               .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,
> +       },
> +       {
> +               .name = "i2s-hifi",
> +               .id = PORT_I2S,
> +               .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_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,
> +                       get_audio_var_t get_audio_var_i,
> +                       set_audio_input_t set_audio_input_i)
> +{
> +       get_audio_var = get_audio_var_i;
> +       set_audio_input = set_audio_input_i;
> +       return snd_soc_register_codec(dev,
> +                               &tda998x_codec_drv,
> +                               tda998x_dais, ARRAY_SIZE(tda998x_dais));
> +}
> +EXPORT_SYMBOL(tda9998x_codec_register);
> +
> +void tda9998x_codec_unregister(struct device *dev)
> +{
> +       snd_soc_unregister_codec(dev);
> +}
> +EXPORT_SYMBOL(tda9998x_codec_unregister);
> +
> +static int __init tda998x_codec_init(void)
> +{
> +       return 0;
> +}
> +static void __exit tda998x_codec_exit(void)
> +{
> +}
> +module_init(tda998x_codec_init);
> +module_exit(tda998x_codec_exit);
> +
> +MODULE_AUTHOR("Jean-Francois Moine <moinejf@free.fr>");
> +MODULE_DESCRIPTION("TDA998X CODEC");
> +MODULE_LICENSE("GPL");
> --
> 2.1.4
> 
>