| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Wed, 1 Oct 2014 17:05:32 +0300
From: Jyri Sarha <jsarha@...com>
To: Jean-Francois Moine <moinejf@...e.fr>,
Mark Brown <broonie@...nel.org>,
Russell King - ARM Linux <linux@....linux.org.uk>
CC: Dave Airlie <airlied@...il.com>,
Andrew Jackson <Andrew.Jackson@....com>,
<alsa-devel@...a-project.org>, <devicetree@...r.kernel.org>,
<dri-devel@...ts.freedesktop.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v6 2/2] drm/i2c:tda998x: Use the HDMI audio CODEC
On 09/24/2014 11:11 AM, Jean-Francois Moine wrote:
> This patch interfaces the HDMI transmitter with the audio system.
>
> Signed-off-by: Jean-Francois Moine <moinejf@...e.fr>
> ---
> .../devicetree/bindings/drm/i2c/tda998x.txt | 18 ++
> drivers/gpu/drm/i2c/Kconfig | 1 +
> drivers/gpu/drm/i2c/tda998x_drv.c | 299
+++++++++++++++++++--
> 3 files changed, 300 insertions(+), 18 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/drm/i2c/tda998x.txt
b/Documentation/devicetree/bindings/drm/i2c/tda998x.txt
> index e9e4bce..e50e7cd 100644
> --- a/Documentation/devicetree/bindings/drm/i2c/tda998x.txt
> +++ b/Documentation/devicetree/bindings/drm/i2c/tda998x.txt
> @@ -17,6 +17,20 @@ Optional properties:
> - video-ports: 24 bits value which defines how the video controller
> output is wired to the TDA998x input - default: <0x230145>
>
> + - audio-ports: must contain one or two values selecting the source
> + in the audio port.
> + The source type is given by the corresponding entry in
> + the audio-port-names property.
> +
> + - audio-port-names: must contain entries matching the entries in
> + the audio-ports property.
> + Each value may be "i2s" or "spdif", giving the type of
> + the audio source.
> +
> + - #sound-dai-cells: must be set to <1> for use with the simple-card.
> + The TDA998x audio CODEC always defines two DAIs.
> + The DAI 0 is the S/PDIF input and the DAI 1 is the I2S input.
> +
> Example:
>
> tda998x: hdmi-encoder {
> @@ -26,4 +40,8 @@ Example:
> interrupts = <27 2>; /* falling edge */
> pinctrl-0 = <&pmx_camera>;
> pinctrl-names = "default";
> +
> + audio-ports = <0x04>, <0x03>;
> + audio-port-names = "spdif", "i2s";
> + #sound-dai-cells = <1>;
> };
> diff --git a/drivers/gpu/drm/i2c/Kconfig b/drivers/gpu/drm/i2c/Kconfig
> index 4d341db..42ca744 100644
> --- a/drivers/gpu/drm/i2c/Kconfig
> +++ b/drivers/gpu/drm/i2c/Kconfig
> @@ -22,6 +22,7 @@ config DRM_I2C_SIL164
> config DRM_I2C_NXP_TDA998X
> tristate "NXP Semiconductors TDA998X HDMI encoder"
> default m if DRM_TILCDC
> + select SND_SOC_HDMI_CODEC
> 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 d476279..66c41c0 100644
> --- a/drivers/gpu/drm/i2c/tda998x_drv.c
> +++ b/drivers/gpu/drm/i2c/tda998x_drv.c
> @@ -20,12 +20,14 @@
> #include <linux/module.h>
> #include <linux/irq.h>
> #include <sound/asoundef.h>
> +#include <linux/platform_device.h>
>
> #include <drm/drmP.h>
> #include <drm/drm_crtc_helper.h>
> #include <drm/drm_encoder_slave.h>
> #include <drm/drm_edid.h>
> #include <drm/i2c/tda998x.h>
> +#include <sound/hdmi.h>
>
> #define DBG(fmt, ...) DRM_DEBUG(fmt"\n", ##__VA_ARGS__)
>
> @@ -44,6 +46,22 @@ struct tda998x_priv {
> wait_queue_head_t wq_edid;
> volatile int wq_edid_wait;
> struct drm_encoder *encoder;
> +
> + /* audio variables */
> + struct platform_device *pdev_codec;
> + u8 audio_ports[2];
> +
> + u8 max_channels; /* EDID parameters */
> + u8 rate_mask;
> + u8 fmt;
> +
> + int audio_sample_format;
> +};
> +
> +struct tda998x_priv2 {
> + struct tda998x_priv base;
> + struct drm_encoder encoder;
> + struct drm_connector connector;
> };
>
> #define to_tda998x_priv(x) ((struct tda998x_priv
*)to_encoder_slave(x)->slave_priv)
> @@ -624,6 +642,8 @@ tda998x_write_avi(struct tda998x_priv *priv,
struct drm_display_mode *mode)
> sizeof(buf));
> }
>
> +/* audio functions */
> +
> static void tda998x_audio_mute(struct tda998x_priv *priv, bool on)
> {
> if (on) {
> @@ -639,12 +659,11 @@ static void
> tda998x_configure_audio(struct tda998x_priv *priv,
> struct drm_display_mode *mode, struct tda998x_encoder_params *p)
> {
> - uint8_t buf[6], clksel_aip, clksel_fs, cts_n, adiv;
> + uint8_t buf[6], clksel_aip, clksel_fs, cts_n, adiv, aclk;
> uint32_t n;
>
> /* Enable audio ports */
> reg_write(priv, REG_ENA_AP, p->audio_cfg);
> - reg_write(priv, REG_ENA_ACLK, p->audio_clk_cfg);
>
> /* Set audio input source */
> switch (p->audio_format) {
> @@ -653,13 +672,29 @@ tda998x_configure_audio(struct tda998x_priv *priv,
> clksel_aip = AIP_CLKSEL_AIP_SPDIF;
> clksel_fs = AIP_CLKSEL_FS_FS64SPDIF;
> cts_n = CTS_N_M(3) | CTS_N_K(3);
> + aclk = 0; /* no clock */
> break;
>
> case AFMT_I2S:
> reg_write(priv, REG_MUX_AP, MUX_AP_SELECT_I2S);
> clksel_aip = AIP_CLKSEL_AIP_I2S;
> clksel_fs = AIP_CLKSEL_FS_ACLK;
> - cts_n = CTS_N_M(3) | CTS_N_K(3);
> +
> + /* with I2S input, the CTS_N predivider depends on
> + * the sample width */
> + switch (priv->audio_sample_format) {
> + case SNDRV_PCM_FORMAT_S16_LE:
> + cts_n = CTS_N_M(3) | CTS_N_K(1);
> + break;
> + case SNDRV_PCM_FORMAT_S24_LE:
> + cts_n = CTS_N_M(3) | CTS_N_K(2);
> + break;
> + default:
Setting the default here does not really help, because
priv->audio_sample_format is initialized to SNDRV_PCM_FORMAT_S24_LE in
tda998x_encoder_set_config(). But I am Ok with the default being
changed for 24 bit samples on i2s interface.
> + case SNDRV_PCM_FORMAT_S32_LE:
> + cts_n = CTS_N_M(3) | CTS_N_K(3);
> + break;
> + }
> + aclk = 1; /* clock enable */
> break;
>
> default:
> @@ -671,6 +706,7 @@ tda998x_configure_audio(struct tda998x_priv *priv,
> reg_clear(priv, REG_AIP_CNTRL_0, AIP_CNTRL_0_LAYOUT |
> AIP_CNTRL_0_ACR_MAN); /* auto CTS */
> reg_write(priv, REG_CTS_N, cts_n);
> + reg_write(priv, REG_ENA_ACLK, aclk);
>
> /*
> * Audio input somehow depends on HDMI line rate which is
> @@ -727,6 +763,144 @@ tda998x_configure_audio(struct tda998x_priv *priv,
> tda998x_write_aif(priv, p);
> }
>
> +/* audio codec interface */
> +
> +/* return the audio parameters extracted from the last EDID */
> +static int tda998x_get_audio(struct device *dev,
> + int *max_channels,
> + int *rate_mask,
> + int *fmt)
> +{
> + struct tda998x_priv2 *priv2 = dev_get_drvdata(dev);
> + struct tda998x_priv *priv = &priv2->base;
> +
> + if (!priv->encoder->crtc)
> + return -ENODEV;
> +
> + *max_channels = priv->max_channels;
> + *rate_mask = priv->rate_mask;
> + *fmt = priv->fmt;
> + return 0;
> +}
> +
> +/* switch the audio port and initialize the audio parameters for
streaming */
> +static void tda998x_audio_switch(struct device *dev,
> + int port_index,
> + unsigned sample_rate,
> + int sample_format)
> +{
> + struct tda998x_priv2 *priv2 = dev_get_drvdata(dev);
> + struct tda998x_priv *priv = &priv2->base;
> + struct tda998x_encoder_params *p = &priv->params;
> +
> + if (!priv->encoder->crtc)
> + return;
> +
> + /*
> + * if port_index is negative (streaming stop),
> + * disable the audio port
> + */
> + if (port_index < 0) {
> + reg_write(priv, REG_ENA_AP, 0);
> + return;
> + }
> +
> + /* if same audio parameters, just enable the audio port */
> + if (p->audio_cfg == priv->audio_ports[port_index] &&
> + p->audio_sample_rate == sample_rate &&
> + priv->audio_sample_format == sample_format) {
> + reg_write(priv, REG_ENA_AP, p->audio_cfg);
> + return;
> + }
> +
> + p->audio_format = port_index;
> + p->audio_cfg = priv->audio_ports[port_index];
> + p->audio_sample_rate = sample_rate;
> + priv->audio_sample_format = sample_format;
> + tda998x_configure_audio(priv, &priv->encoder->crtc->hwmode, p);
> +}
> +
> +#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 = AFMT_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,
> + },
> + },
> + {
> + .name = "i2s-hifi",
> + .id = AFMT_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,
> + },
> + },
> +};
> +
> +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_driver = {
> + .dapm_widgets = tda998x_widgets,
> + .num_dapm_widgets = ARRAY_SIZE(tda998x_widgets),
> + .dapm_routes = tda998x_routes,
> + .num_dapm_routes = ARRAY_SIZE(tda998x_routes),
> +};
> +
> +static struct hdmi_data tda998x_hdmi_data = {
> + .get_audio = tda998x_get_audio,
> + .audio_switch = tda998x_audio_switch,
> + .ndais = ARRAY_SIZE(tda998x_dais),
> + .dais = tda998x_dais,
> + .driver = &tda998x_codec_driver,
> +};
> +
> +static void tda998x_create_audio_codec(struct tda998x_priv *priv)
> +{
> + struct platform_device *pdev;
> + struct module *module;
> +
> + request_module("snd-soc-hdmi-codec");
> + pdev = platform_device_register_resndata(&priv->hdmi->dev,
> + "hdmi-audio-codec",
> + PLATFORM_DEVID_NONE,
> + NULL, 0,
> + &tda998x_hdmi_data,
> + sizeof tda998x_hdmi_data);
> + if (IS_ERR(pdev)) {
> + dev_err(&priv->hdmi->dev, "cannot create codec: %ld\n",
> + PTR_ERR(pdev));
> + return;
> + }
> +
> + priv->pdev_codec = pdev;
> + module = pdev->dev.driver->owner;
> + if (module)
> + try_module_get(module);
> +}
> +
> /* DRM encoder functions */
>
> static void tda998x_encoder_set_config(struct tda998x_priv *priv,
> @@ -746,6 +920,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_ports[p->audio_format] = p->audio_cfg;
> + priv->audio_sample_format = SNDRV_PCM_FORMAT_S24_LE;
See the previous comment.
> }
>
> static void tda998x_encoder_dpms(struct tda998x_priv *priv, int mode)
> @@ -1128,6 +1304,47 @@ fail:
> return NULL;
> }
>
> +static void tda998x_set_audio(struct tda998x_priv *priv,
> + struct drm_connector *connector)
> +{
> + u8 *eld = connector->eld;
> + u8 *sad;
> + int sad_count;
> + unsigned eld_ver, mnl, rate_mask;
> + unsigned max_channels, 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 = 2;
> + rate_mask = 0;
> + fmt = 0;
> + while (sad_count--) {
> + switch (sad[0] & 0x78) {
> + case 0x08: /* PCM */
> + max_channels = max(max_channels, (sad[0] & 7) + 1u);
> + rate_mask |= sad[1];
> + fmt |= sad[2] & 0x07;
> + break;
> + }
> + sad += 3;
> + }
> +
> + priv->max_channels = max_channels;
> + priv->rate_mask = rate_mask;
> + priv->fmt = fmt;
> +}
> +
> static int
> tda998x_encoder_get_modes(struct tda998x_priv *priv,
> struct drm_connector *connector)
> @@ -1139,6 +1356,12 @@ 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);
> +
> + /* set the audio parameters from the EDID */
> + if (priv->is_hdmi_sink) {
> + drm_edid_to_eld(connector, edid);
> + tda998x_set_audio(priv, connector);
> + }
> kfree(edid);
> }
>
> @@ -1167,12 +1390,19 @@ tda998x_encoder_set_property(struct
drm_encoder *encoder,
>
> static void tda998x_destroy(struct tda998x_priv *priv)
> {
> + struct module *module;
> +
> /* disable all IRQs and free the IRQ handler */
> cec_write(priv, REG_CEC_RXSHPDINTENA, 0);
> reg_clear(priv, REG_INT_FLAGS_2, INT_FLAGS_2_EDID_BLK_RD);
> if (priv->hdmi->irq)
> free_irq(priv->hdmi->irq, priv);
>
> + if (priv->pdev_codec) {
> + module = priv->pdev_codec->dev.driver->owner;
> + module_put(module);
> + platform_device_del(priv->pdev_codec);
> + }
> i2c_unregister_device(priv->cec);
> }
>
> @@ -1254,12 +1484,16 @@ static int tda998x_create(struct i2c_client
*client, struct tda998x_priv *priv)
> {
> struct device_node *np = client->dev.of_node;
> u32 video;
> - int rev_lo, rev_hi, ret;
> + int i, j, rev_lo, rev_hi, ret;
> + const char *p;
>
> priv->vip_cntrl_0 = VIP_CNTRL_0_SWAP_A(2) | VIP_CNTRL_0_SWAP_B(3);
> 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;
> priv->cec = i2c_new_dummy(client->adapter, 0x34);
> @@ -1351,17 +1585,48 @@ 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 device tree parameters */
> + if (np) {
>
> - /* 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;
> + /* 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;
> + }
> +
> + /* audio properties */
> + for (i = 0; i < 2; i++) {
> + u32 port;
> +
> + ret = of_property_read_u32_index(np, "audio-ports", i, &port);
> + if (ret)
> + break;
> + ret = of_property_read_string_index(np, "audio-port-names",
> + i, &p);
> + if (ret) {
> + dev_err(&client->dev,
> + "missing audio-port-names[%d]\n", i);
> + break;
> + }
> + if (strcmp(p, "spdif") == 0) {
> + j = AFMT_SPDIF;
> + } else if (strcmp(p, "i2s") == 0) {
> + j = AFMT_I2S;
> + } else {
> + dev_err(&client->dev,
> + "bad audio-port-names '%s'\n", p);
> + break;
> + }
> + priv->audio_ports[j] = port;
> + }
> }
>
> + /* create the audio CODEC */
> + if (priv->audio_ports[AFMT_SPDIF] || priv->audio_ports[AFMT_I2S])
> + tda998x_create_audio_codec(priv);
> +
> return 0;
>
> fail:
> @@ -1395,15 +1660,13 @@ static int tda998x_encoder_init(struct
i2c_client *client,
> encoder_slave->slave_priv = priv;
> encoder_slave->slave_funcs = &tda998x_encoder_slave_funcs;
>
> + /* set the drvdata pointer to priv2 for CODEC calls */
> + dev_set_drvdata(&client->dev,
> + container_of(priv, struct tda998x_priv2, base));
> +
> return 0;
> }
>
> -struct tda998x_priv2 {
> - struct tda998x_priv base;
> - struct drm_encoder encoder;
> - struct drm_connector connector;
> -};
> -
> #define conn_to_tda998x_priv2(x) \
> container_of(x, struct tda998x_priv2, connector);
>
>
The only audio side change in the platform data usage of tda998x_drv I
can see is the change in the default value of CTS_N.
Best regards,
Jyri
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists