Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751721AbaJAOF7 (ORCPT ); Wed, 1 Oct 2014 10:05:59 -0400 Received: from bear.ext.ti.com ([192.94.94.41]:57218 "EHLO bear.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751208AbaJAOF5 (ORCPT ); Wed, 1 Oct 2014 10:05:57 -0400 Message-ID: <542C0A2C.6010802@ti.com> Date: Wed, 1 Oct 2014 17:05:32 +0300 From: Jyri Sarha User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.1.2 MIME-Version: 1.0 To: Jean-Francois Moine , Mark Brown , Russell King - ARM Linux CC: Dave Airlie , Andrew Jackson , , , , Subject: Re: [PATCH v6 2/2] drm/i2c:tda998x: Use the HDMI audio CODEC References: <4b3d35a14461ed164956b7f5aa77b29170bc393d.1411547014.git.moinejf@free.fr> In-Reply-To: <4b3d35a14461ed164956b7f5aa77b29170bc393d.1411547014.git.moinejf@free.fr> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 > --- > .../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 > #include > #include > +#include > > #include > #include > #include > #include > #include > +#include > > #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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/