[PATCH v9 3/4] ASoC: tda998x: add a codec to the HDMI transmitter
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
> ---
[snip]
> +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) */
[...]
> + snd_pcm_hw_constraint_minmax(runtime,
> + SNDRV_PCM_HW_PARAM_CHANNELS,
> + 1, sad[SAD_MX_CHAN_I]);
In the light of our discussions elsewhere [1], shouldn't this
be constrained by the number of hardware channels that the TDA998x
supports too? That is, the maximum number of channels should
be the lesser of sd[SAD_MX_CHAN_I] and number_of_I2S channels
(or S/PDIF channels if so configured).
Andrew
[1]
http://mailman.alsa-project.org/pipermail/alsa-devel/2015-January/086090.html
[PATCH v9 1/4] drm/i2c: tda998x: Add DT support for audio
On 01/09/15 13:07, Russell King - ARM Linux wrote: > On Fri, Jan 09, 2015 at 01:54:01PM +0100, Jean-Francois Moine wrote: >> On Fri, 9 Jan 2015 11:45:29 + >> Russell King - ARM Linux wrote: >>> I think we need to understand exactly how the 998x map I2S inputs to the >>> HDMI channels to avoid making a mistake with the binding; remember, the >>> binding isn't something that can be easily "bug fixed" at a later date >>> as anything we come up with now has to be supported long term by the >>> kernel. >> >> The DT describes the hardware configuration. > > You're missing my point. > > How does the driver know which of the I2S pins to enable in I2S mode? [snip] > My question is: how do we know which I2S inputs to enable, or are > you suggesting that all I2S inputs should be enabled if operating in > I2S mode irrespective of whether they may be active? > Isn't it the case that for I2S: * Word Select (WS) is always required to disambiguate left/right. So WS need not be specified in any device tree configuration. * The audio inputs depend on a particular board but at least one is required. Fortunately, the TDA998x devices have all the same pin/input numbering so they /could/ be described as i2s0, i2s1, i2s2 and i2s3 as per J-F's earlier email. But tt isn't clear from my reading of the TDA19988 datasheet (for example) whether one can skip channels (so does channel 0 always have to be present?). If the channels must be enabled from 0 then one could simply specify the number of inputs. (The datasheets that I have don't indicate whether there's any channel remapping performed internally by the TDA998x). * What the driver does need to take account of is the number of inputs that are active so that the sound CODEC can be properly configured. Andrew
[PATCH v9 1/4] drm/i2c: tda998x: Add DT support for audio
On 01/08/15 20:04, Mark Brown wrote: > On Thu, Jan 08, 2015 at 05:42:57PM +0100, Jean-Francois Moine wrote: > >> Examples: > >> - for the Cubox: > >> audio-inputs = "i2s", "spdif"; > >> - for some other board with I2S on the pins 3 and 4 only: > >> audio-inputs = "none", "none", "i2s", "i2s"; > >> - for a fully wired TDA9983B (no driver yet): > >> audio-inputs = "i2s", "i2s", "i2s", "i2s", "spdif"; > > I think that mostly works, though I do wonder if we need a way to > specify the ordering of the pins (if you can make pin 3 be the first two > I2S channels for example)? Someone might choose a strange mapping for > board routing reasons for example. > If it helps, I've collated the pin assignments given in the various TDA datasheets that I can find: Chip> 9983B 998919988 19989 Mode> - S/PDIF I2S S/PDIF I2S S/PDIF I2S Pin AP0 WS - WS - WS - WS AP1 I2S#0 S/PDIF I2S#0 S/PDIF I2S#0 S/PDIF I2S#0 AP2 I2S#1 - - S/PDIF I2S#1 S/PDIF I2S#1 AP3 I2S#2 - - - I2S#2* MCLK- AP4 I2S#3 - - - I2S#3* - - AP5 MCLK- - - - - - AP6 S/PDIF - - - - - - AP7 AUX - - - - - - WS = I2S Word Select * Depends on package The 9983B differs from the other devices in that the I2S and S/PDIF functionality is not multiplexed onto various pins. Andrew
[PATCH v9 1/4] drm/i2c: tda998x: Add DT support for audio
On 01/07/15 17:08, Jean-Francois Moine wrote: > On Wed, 07 Jan 2015 14:39:13 + > Andrew Jackson wrote: > >>> + - 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. >> >> I think that this entry might benefit from a little more explanation. >> The value specified here selects which pins on the chip provide the >> audio input doesn't it? In the outline datasheet that I have these are >> listed in table 17: >> >> Audio port Input configuration >> S/PDIF I2S-bus >> AP0 - WS (word select) >> AP1 S/PDIF inputI2S-bus channel 0 >> AP2 S/PDIF inputI2S-bus channel 1 >> AP3[1] I2S-bus channel 2 >> AP4[1] I2S-bus channel 3 >> ACLK - SCK (I2S-bus clock) >> >> [1] Depending on package. > > Your table is close to the one in the TDA9983B documentation I have, > but the pins are not exactly the same: > > AP0 WS (word select) > AP1 I2S-bus port 0 > AP2 I2S-bus port 1 > AP3 I2S-bus port 2 > AP4 I2S-bus port 3 > AP5 MCLK (master clock for S/PDIF) > AP6 S/PDIF input > AP7 AUX (internal test) > ACLK SCK (I2S-bus clock) > > That's why I did not know clearly why I had to set AP2 for S/PDIF input > and (AP0 + AP1) for I2S input in the Cubox. > > Then, the only more explanation I could give is "have a look at the > audio input format and at the register 0x1e page 0 in the documentation > of the TDA998x chip". > > BTW, the tda998x driver supports only the TDA9989, TDA19988 and > TDA19989 chips. If the TDA9983B would be supported, the audio port > definitions would be of no use. > > So, what would you see as an explanation? > I understand your difficulty! I was just wanting something to clarify the meaning of the value without reference to the driver source. You could add something like this to your existing explanation: "The value describes which audio input pins are selected; this varies depending on chip type so consult the section on audio port configuration in the relevant datasheet.". Andrew
[PATCH v9 3/4] ASoC: tda998x: add a codec to the HDMI transmitter
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
> ---
> 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
> #include
> #include
> +#include
> +
> +#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 = >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 = >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, >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;
> +
> +
[PATCH v9 1/4] drm/i2c: tda998x: Add DT support for audio
On 01/07/15 09:10, Jean-Francois Moine wrote:
> This patch permits the definition of the audio ports from the device tree.
>
> Signed-off-by: Jean-Francois Moine
> ---
> .../devicetree/bindings/drm/i2c/tda998x.txt| 18 +++
> drivers/gpu/drm/i2c/tda998x_drv.c | 60
> ++
> 2 files changed, 69 insertions(+), 9 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.
I think that this entry might benefit from a little more explanation.
The value specified here selects which pins on the chip provide the
audio input doesn't it? In the outline datasheet that I have these are
listed in table 17:
Audio port Input configuration
S/PDIF I2S-bus
AP0 - WS (word select)
AP1 S/PDIF inputI2S-bus channel 0
AP2 S/PDIF inputI2S-bus channel 1
AP3[1] I2S-bus channel 2
AP4[1] I2S-bus channel 3
ACLK- SCK (I2S-bus clock)
[1] Depending on package.
Andrew
> +
> + - 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 = <_camera>;
> pinctrl-names = "default";
> +
> + audio-ports = <0x04>, <0x03>;
> + audio-port-names = "spdif", "i2s";
> + #sound-dai-cells = <1>;
> };
> diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c
> b/drivers/gpu/drm/i2c/tda998x_drv.c
> index 70658af..9d9b072 100644
> --- a/drivers/gpu/drm/i2c/tda998x_drv.c
> +++ b/drivers/gpu/drm/i2c/tda998x_drv.c
> @@ -20,6 +20,7 @@
> #include
> #include
> #include
> +#include
>
> #include
> #include
> @@ -44,6 +45,8 @@ struct tda998x_priv {
> wait_queue_head_t wq_edid;
> volatile int wq_edid_wait;
> struct drm_encoder *encoder;
> +
> + u8 audio_ports[2];
> };
>
> #define to_tda998x_priv(x) ((struct tda998x_priv
> *)to_encoder_slave(x)->slave_priv)
> @@ -1254,12 +1257,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, 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,15 +1358,50 @@ 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) {
> +
> + /* optional video properties */
> + ret = of_property_read_u32(np, "video-ports", );
> + if (ret == 0) {
> + priv->vip_cntrl_0 = video >> 16;
> + priv->vip_cntrl_1 = video >> 8;
> + priv->vip_cntrl_2 = video;
> + }
> +
> + /* optional audio properties */
> + for (i = 0; i < 2; i++) {
> + u32 port;
>
> - /* get the optional video properties */
> - ret = of_property_read_u32(np, "video-ports", );
> - if (ret == 0) {
> - priv->vip_cntrl_0 = video >> 16;
> - priv->vip_cntrl_1 = video >> 8;
> - priv->vip_cntrl_2 = video;
> + ret = of_property_read_u32_index(np,
[PATCH] drm: i2c: tda998x: Retry fetching the EDID if it fails first time.
On 11/29/14 10:41, Jean-Francois Moine wrote: > On Fri, 28 Nov 2014 09:02:39 + > Andrew Jackson wrote: > >>> It seems that your patch is deprecated by Laurent Pinchart's >>> [PATCH] drm: tda998x: Use drm_do_get_edid() >>> http://lists.freedesktop.org/archives/dri-devel/2014-November/072906.html >>> >> >> Thank you for the heads-up, I'd not seen that. I'll consider how my patch >> might be modified to suit the new infrastructure. > > You don't need any patch: drm_do_get_edid() already loops on reading > the EDID. > Thank you again: that saves me work! Andrew
[PATCH] drm: i2c: tda998x: Retry fetching the EDID if it fails first time.
On 11/28/14 08:19, Jean-Francois Moine wrote: > On Tue, 18 Nov 2014 17:41:26 + > Andrew Jackson wrote: > >> Fetching the EDID from a connected monitor is an automated thing >> with NXP TDA19988. But on some boards the fetching fails for the >> first time silently without any indication that an error has occured. >> More than that, subsequent fetches of the EDID succeed until the >> monitor(s) are unplugged. >> >> Add a function to validate the read EDID and retry if the block >> retrieved is not valid. >> >> Signed-off-by: Andrew Jackson >> Signed-off-by: Liviu Dudau > > It seems that your patch is deprecated by Laurent Pinchart's > [PATCH] drm: tda998x: Use drm_do_get_edid() > http://lists.freedesktop.org/archives/dri-devel/2014-November/072906.html > Thank you for the heads-up, I'd not seen that. I'll consider how my patch might be modified to suit the new infrastructure. Andrew
[PATCH] drm/i2c: tda998x: Allow for different audio sample rates
On 11/18/14 18:00, Russell King - ARM Linux wrote: > On Tue, Nov 18, 2014 at 05:39:30PM +0000, Andrew Jackson wrote: >> On HDMI, the audio data are carried across the HDMI link which is >> driven by the TDMS clock. The TDMS clock is dependent on the video pixel >> rate. >> >> This patch sets the denominator (Cycle Time Stamp) appropriately >> allowing the driver to send audio to a wider range of HDMI sinks >> (i.e. monitors). > > This is actually pointless, because we don't use "manual" CTS mode. > > If the clocks for the video and audio are coherent, then you can program > both the N and CTS values to allow the sink to properly recover the > synchronous audio clock. > > However, in most cases, the audio and video clocks are not coherent, and > since the recovered audio clock has to match the source audio clock, the > only way this can be done is by the TDA998x (or in fact other HDMI > encoder) to measure the audio clock rate and generate the CTS value > itself. > > This is the mode we drive the TDA998x - so the programmed CTS value is > irrelevant. My apologies for the noise: I originally created the patch when one of the monitors with which I was working wouldn't play sound as expected. However, I now find that the monitor plays sound with or without the patch so it must have been something else. I'd missed the significance of the "auto CTS" comment a few lines earlier (partly because I've no datasheet on the TDA998x). Andrew
[PATCH] drm: i2c: tda998x: Retry fetching the EDID if it fails first time.
Fetching the EDID from a connected monitor is an automated thing
with NXP TDA19988. But on some boards the fetching fails for the
first time silently without any indication that an error has occured.
More than that, subsequent fetches of the EDID succeed until the
monitor(s) are unplugged.
Add a function to validate the read EDID and retry if the block
retrieved is not valid.
Signed-off-by: Andrew Jackson
Signed-off-by: Liviu Dudau
---
drivers/gpu/drm/i2c/tda998x_drv.c | 36
1 files changed, 28 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c
b/drivers/gpu/drm/i2c/tda998x_drv.c
index d476279..025adc0 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -1066,11 +1066,36 @@ static int read_edid_block(struct tda998x_priv *priv,
uint8_t *buf, int blk)
return 0;
}
+static int
+read_validate_edid_block(struct tda998x_priv *priv, uint8_t *buf, int blk)
+{
+ bool print_bad_edid = drm_debug & DRM_UT_KMS;
+ int ret;
+ int retries = 1;
+
+ do
+ {
+ bool print_bad = print_bad_edid && (retries == 0);
+
+ ret = read_edid_block(priv, buf, blk);
+ /* Fail on I2C error */
+ if (ret)
+ break;
+
+ /* But retry checksum errored blocks */
+ if (drm_edid_block_valid(buf, blk, print_bad))
+ break;
+ else
+ ret = -EINVAL;
+ } while (retries-- > 0);
+
+ return ret;
+}
+
static uint8_t *do_get_edid(struct tda998x_priv *priv)
{
int j, valid_extensions = 0;
uint8_t *block, *new;
- bool print_bad_edid = drm_debug & DRM_UT_KMS;
if ((block = kmalloc(EDID_LENGTH, GFP_KERNEL)) == NULL)
return NULL;
@@ -1079,10 +1104,7 @@ static uint8_t *do_get_edid(struct tda998x_priv *priv)
reg_clear(priv, REG_TX4, TX4_PD_RAM);
/* base block fetch */
- if (read_edid_block(priv, block, 0))
- goto fail;
-
- if (!drm_edid_block_valid(block, 0, print_bad_edid))
+ if (read_validate_edid_block(priv, block, 0))
goto fail;
/* if there's no extensions, we're done */
@@ -1096,10 +1118,8 @@ static uint8_t *do_get_edid(struct tda998x_priv *priv)
for (j = 1; j <= block[0x7e]; j++) {
uint8_t *ext_block = block + (valid_extensions + 1) *
EDID_LENGTH;
- if (read_edid_block(priv, ext_block, j))
- goto fail;
- if (!drm_edid_block_valid(ext_block, j, print_bad_edid))
+ if (read_validate_edid_block(priv, ext_block, j))
goto fail;
valid_extensions++;
--
1.7.1
[PATCH] drm/i2c: tda998x: Allow for different audio sample rates
On HDMI, the audio data are carried across the HDMI link which is
driven by the TDMS clock. The TDMS clock is dependent on the video pixel
rate.
This patch sets the denominator (Cycle Time Stamp) appropriately
allowing the driver to send audio to a wider range of HDMI sinks
(i.e. monitors).
Signed-off-by: Andrew Jackson
---
drivers/gpu/drm/i2c/tda998x_drv.c | 22 ++
1 files changed, 18 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c
b/drivers/gpu/drm/i2c/tda998x_drv.c
index d476279..da0d504 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -640,7 +640,7 @@ 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;
- uint32_t n;
+ uint32_t n, cts;
/* Enable audio ports */
reg_write(priv, REG_ENA_AP, p->audio_cfg);
@@ -696,9 +696,23 @@ tda998x_configure_audio(struct tda998x_priv *priv,
n = 128 * p->audio_sample_rate / 1000;
/* Write the CTS and N values */
- buf[0] = 0x44;
- buf[1] = 0x42;
- buf[2] = 0x01;
+ if ((n > 0) && (mode->clock > 0)) {
+ /*
+* For non-coherent clocks, the average CTS value is
+* calculated as:
+* fTMDS * n / (128 * fs)
+* which simplifies to:
+* fTMDS / 1000
+* (See sections 7.2.2 and 7.2.3 of the HDMI specification.)
+* NB mode->clock is in kHz.
+*/
+ cts = mode->clock;
+ } else {
+ cts = 82500;
+ }
+ buf[0] = cts;
+ buf[1] = cts >> 8;
+ buf[2] = cts >> 16;
buf[3] = n;
buf[4] = n >> 8;
buf[5] = n >> 16;
--
1.7.1
[PATCH v2] drm/i2c: tda998x: Set the CEC I2C address based on the slave I2C address.
The I2C address for the TDA9989 and TDA19989 is fixed at 0x34 but the two LSBs of the TDA19988's address are set by two configuration pins on the chip. Irrespective of the chip, the associated CEC peripheral's I2C address is based upon the main I2C address. This patch avoids any special handling required to support systems that contain multiple TDA19988 devices on the same I2C bus. Signed-off-by: Andrew Jackson Signed-off-by: Liviu Dudau --- drivers/gpu/drm/i2c/tda998x_drv.c |5 - 1 files changed, 4 insertions(+), 1 deletions(-) diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c index d476279..eeab17a 100644 --- a/drivers/gpu/drm/i2c/tda998x_drv.c +++ b/drivers/gpu/drm/i2c/tda998x_drv.c @@ -1255,6 +1255,7 @@ 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; + unsigned short cec_addr; 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); @@ -1262,7 +1263,9 @@ static int tda998x_create(struct i2c_client *client, struct tda998x_priv *priv) priv->current_page = 0xff; priv->hdmi = client; - priv->cec = i2c_new_dummy(client->adapter, 0x34); + /* CEC I2C address bound to TDA998x I2C addr by configuration pins */ + cec_addr = 0x34 + (client->addr & 0x03); + priv->cec = i2c_new_dummy(client->adapter, cec_addr); if (!priv->cec) return -ENODEV; -- 1.7.1
[PATCH v6 1/2] ASoC:codecs: Add a transmitter interface to the HDMI CODEC CODEC
On 09/24/14 08:49, Jean-Francois Moine wrote:
> The audio constraints of the HDMI interface are defined by the EDID
> which is sent by the connected device.
>
> The HDMI transmitters may have one or many audio sources.
>
> This patch adds two functions to the HDMI CODEC:
> - it updates the audio constraints from the EDID,
> - it gives the audio source type to the HDMI transmitter
> on start of audio streaming.
>
> Signed-off-by: Jean-Francois Moine
> ---
> include/sound/hdmi.h| 20 ++
> sound/soc/codecs/hdmi.c | 174
> ++--
> 2 files changed, 188 insertions(+), 6 deletions(-)
> create mode 100644 include/sound/hdmi.h
>
> diff --git a/include/sound/hdmi.h b/include/sound/hdmi.h
> new file mode 100644
> index 000..49062c7
> --- /dev/null
> +++ b/include/sound/hdmi.h
> @@ -0,0 +1,20 @@
> +#ifndef SND_HDMI_H
> +#define SND_HDMI_H
> +
> +#include
> +
> +/* platform_data */
> +struct hdmi_data {
> + int (*get_audio)(struct device *dev,
> + int *max_channels,
> + int *rate_mask,
> + int *fmt);
> + void (*audio_switch)(struct device *dev,
> + int port_index,
> + unsigned sample_rate,
> + int sample_format);
> + int ndais;
> + struct snd_soc_dai_driver *dais;
> + struct snd_soc_codec_driver *driver;
> +};
> +#endif
> diff --git a/sound/soc/codecs/hdmi.c b/sound/soc/codecs/hdmi.c
> index 1087fd5..6ea2772 100644
> --- a/sound/soc/codecs/hdmi.c
> +++ b/sound/soc/codecs/hdmi.c
> @@ -22,9 +22,146 @@
> #include
> #include
> #include
> +#include
> +#include
>
> #define DRV_NAME "hdmi-audio-codec"
>
> +struct hdmi_priv {
> + struct hdmi_data hdmi_data;
> + struct snd_pcm_hw_constraint_list rate_constraints;
> +};
> +
> +static int hdmi_dev_match(struct device *dev, void *data)
> +{
> + return !strcmp(dev_name(dev), (char *) data);
> +}
> +
> +/* get the codec device */
> +static struct device *hdmi_get_cdev(struct device *dev)
> +{
> + struct device *cdev;
> +
> + cdev = device_find_child(dev,
> + DRV_NAME,
> + hdmi_dev_match);
> + if (!cdev)
> + dev_err(dev, "Cannot get codec device");
> + else
> + put_device(cdev);
> + return cdev;
> +}
> +
> +static int hdmi_startup(struct snd_pcm_substream *substream,
> + struct snd_soc_dai *dai)
> +{
> + struct snd_pcm_runtime *runtime = substream->runtime;
> + struct device *cdev;
> + struct hdmi_priv *priv;
> + struct snd_pcm_hw_constraint_list *rate_constraints;
> + int ret, max_channels, rate_mask, fmt;
> + u64 formats;
> + static const u32 hdmi_rates[] = {
> + 32000, 44100, 48000, 88200, 96000, 176400, 192000
> + };
> +
> + cdev = hdmi_get_cdev(dai->dev);
> + if (!cdev)
> + return -ENODEV;
> + priv = dev_get_drvdata(cdev);
It might be a good idea to qualify priv (and perhaps get_audio and
audio_switch) here? This might be that the relevant (function) pointers are
non-NULL.
Andrew
> +
> + /* get the EDID values and the rate constraints buffer */
> + ret = priv->hdmi_data.get_audio(dai->dev,
> + _channels, _mask, );
> + if (ret < 0)
> + return ret; /* no screen */
> +
> + /* convert the EDID values to audio constraints */
> + rate_constraints = >rate_constraints;
> + rate_constraints->list = hdmi_rates;
> + rate_constraints->count = ARRAY_SIZE(hdmi_rates);
> + rate_constraints->mask = rate_mask;
> + snd_pcm_hw_constraint_list(runtime, 0,
> +SNDRV_PCM_HW_PARAM_RATE,
> +rate_constraints);
> +
> + formats = 0;
> + if (fmt & 1)
> + formats |= SNDRV_PCM_FMTBIT_S16_LE;
> + if (fmt & 2)
> + formats |= SNDRV_PCM_FMTBIT_S20_3LE;
> + if (fmt & 4)
> + formats |= SNDRV_PCM_FMTBIT_S24_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, max_channels);
> + return 0;
> +}
> +
> +static int hdmi_hw_params(struct snd_pcm_substream *substream,
> + struct snd_pcm_hw_params *params,
> + struct snd_soc_dai *dai)
> +{
> + struct device *cdev;
> + struct hdmi_priv *priv;
> +
> + cdev = hdmi_get_cdev(dai->dev);
> + if (!cdev)
> + return -ENODEV;
> + priv = dev_get_drvdata(cdev);
> +
> + priv->hdmi_data.audio_switch(dai->dev, dai->id,
> + params_rate(params),
> +
[PATCH v3] ASoC: tda998x: add a codec to the HDMI transmitter
On 07/08/14 10:40, Jean-Francois Moine wrote: > This patch adds a CODEC function to the NXP TDA998x HDMI transmitter. > > The CODEC handles both I2S and S/PDIF input and does dynamic input > switch in the TDA998x I2C driver on start/stop audio streaming. > > Signed-off-by: Jean-Francois Moine > --- Shouldn't the patch also capture the current audio rate in tda998x_codec.c:tda_hw_params? Otherwise, when tda998x_drv.c:tda998x_audio_start invokes tda998x_drv.c:tda998x_configure_audio, the value of 'N' will be calculated incorrectly. So something similar to: diff --git a/drivers/gpu/drm/i2c/tda998x_codec.c b/drivers/gpu/drm/i2c/tda998x_codec.c index e7f223d..b6c4fb3 100755 --- a/drivers/gpu/drm/i2c/tda998x_codec.c +++ b/drivers/gpu/drm/i2c/tda998x_codec.c @@ -111,6 +111,7 @@ static int tda_hw_params(struct snd_pcm_substream *substream, tda998x_audio_start(priv, 0); return 0; } + priv->params.audio_sample_rate = params_rate(params); priv->params.audio_format = dai->id; priv->audio_sample_format = params_format(params); params.audio_cfg = Andrew
[alsa-devel] [PATCH v2] ASoC: tda998x: add a codec to the HDMI transmitter
On 07/04/14 08:17, Jean-Francois Moine wrote:
> This patch adds a CODEC function to the NXP TDA998x HDMI transmitter.
>
> The CODEC handles both I2S and S/PDIF input and does dynamic input
> switch in the TDA998x I2C driver on start/stop audio streaming.
>
> Signed-off-by: Jean-Francois Moine
> ---
[snip]
> +++ b/drivers/gpu/drm/i2c/tda998x_codec.c
[snip]
> +static int tda_startup(struct snd_pcm_substream *substream,
> + struct snd_soc_dai *dai)
> +{
> + struct tda998x_priv *priv = snd_soc_codec_get_drvdata(dai->codec);
> + u8 *eld = priv->eld;
> + struct snd_pcm_runtime *runtime = substream->runtime;
> + u8 *sad;
> + int sad_count;
> + unsigned eld_ver, mnl, rate_mask;
> + unsigned max_channels, fmt;
> + u64 formats;
> + struct snd_pcm_hw_constraint_list *rate_constraints =
> + >rate_constraints;
> + static const u32 hdmi_rates[] = {
> + 32000, 44100, 48000, 88200, 9600, 176400, 192000
> + };
> +
Shouldn't this be 96000, not 9600? Assuming that the table is ordered in terms
of increasing frequencies.
Andrew
