[PATCH] drm/i2c: tda998x: Allow for different audio sample rates

[PATCH] drm/i2c: tda998x: Allow for different audio sample rates

[Andrew Jackson]

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 <Andrew.Jackson@arm.com>
---
 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

Re: [PATCH] drm/i2c: tda998x: Allow for different audio sample rates

[Russell King - ARM Linux]

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.

See the HDMI spec, section 7.2 for a discussion about this, especially
non-coherent clocks.

-- 
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.

Re: [PATCH] drm/i2c: tda998x: Allow for different audio sample rates

[Andrew Jackson]

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