On Fri, Feb 22, 2019 at 04:27:35PM +0000, Russell King - ARM Linux admin wrote:
On Fri, Feb 22, 2019 at 10:47:35AM -0500, Sven Van Asbroeck wrote:
The config structure which you need to fill in to init the audio has a "i2s qualifier" field, where you have the choice between 16 and 32 bits. This then maps to a "Clock Time Stamp factor x" called CTSX, which maps to the following CTS_N register settings:
CTSX -> CTS_N (m,k)
16 -> (3,0) 32 -> (3,1) (i2s qualifier = 16 bits) 48 -> (3,2) 64 -> (3,3) (i2s qualifier = 32 bits) 128 -> (0,0)
Okay, this is my hypothesis about how the TDA998x works:
In the HDMI spec, the CTS value is generated using:
128*fS ---- divide ---> CTS counter -----> CTS value (clock) ^ ^ | | fTMDS_clock -----------------+------------> TMDS clock | N value -------+--------------------------> N value
What this does is count the number of TMDS clocks for every output of the divider to produce a value for CTS, effectively implementing
CTS = fTMDS_clock·N / 128·fS_source
The sink regenerates the 128·fS clock at the sink by reversing the process - this is the equation given in the HDMI spec:
128·fS_sink = fTMDS_clock·N / CTS
Using the "actual" values for 'm' and 'k' rather than the register values, if we subsitute BCLK for the 128·fS input, assume that instead of a CTS counter it is the mts counter which has to be divided by 'm' to get the CTS value, and take account of 'k' as an extra prescaler, what we end up with is:
mts = fTMDS_clock·N·k / BCLK CTS = fTMDS_clock·N·k / (BCLK·m)
Throw this into the reverse process, and we end up with:
128·fS_sink = BCLK·m / k
From the table of values you've given above, the CTSX value looks very
much like the BCLK fS ratio. For the 64·fS ratio, we have m=8 (reg value 3) k=4 (reg value 3):
BCLK ratio m k 128·fS_sink is in terms of fS_source 16 8 1 BCLK·8 128·fS_source 32 8 2 BCLK·4 128·fS_source 48 8 3 BCLK·8/3 128·fS_source 64 8 4 BCLK·2 128·fS_source 128 1 1 BCLK 128·fS_source
What this shows is that we end up with the same sample rate on the sink as the source despite the different BCLK ratios with this assumption.
What we also know is that SPDIF uses a 64·fS clock, and is programmed with m=8 k=4, which corresponds nicely with the above.
In light of that, what about this, which rejigs the driver to use a bclk_ratio rather than the sample width. We then just need to work out what to do about getting the bclk_ratio value into the driver in a way that we don't end up breaking existing users.
A possible solution to that would be for hdmi-codec to default that to zero unless it's been definitively provided by the ASoC "card", which would allow the old behaviour of selecting the CTS_N M/K values depending on the sample width, which we know works for some people.
diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c index 9300469dbec0..3d5eb5024b2c 100644 --- a/drivers/gpu/drm/i2c/tda998x_drv.c +++ b/drivers/gpu/drm/i2c/tda998x_drv.c @@ -930,21 +930,26 @@ tda998x_configure_audio(struct tda998x_priv *priv, reg_write(priv, REG_MUX_AP, MUX_AP_SELECT_I2S); clksel_aip = AIP_CLKSEL_AIP_I2S; clksel_fs = AIP_CLKSEL_FS_ACLK; - switch (params->sample_width) { + switch (params->bclk_ratio) { case 16: + cts_n = CTS_N_M(3) | CTS_N_K(0); + break; + case 32: cts_n = CTS_N_M(3) | CTS_N_K(1); break; - case 18: - case 20: - case 24: + case 48: cts_n = CTS_N_M(3) | CTS_N_K(2); break; - default: - case 32: + case 64: cts_n = CTS_N_M(3) | CTS_N_K(3); break; + case 128: + cts_n = CTS_N_M(0) | CTS_N_K(0); + break; + default: + dev_err(&priv->hdmi->dev, "unsupported I2S bclk ratio\n"); + return -EINVAL; } - switch (params->format & AFMT_I2S_MASK) { case AFMT_I2S_LEFT_J: i2s_fmt = I2S_FORMAT_LEFT_J; @@ -1040,6 +1045,22 @@ static int tda998x_audio_hw_params(struct device *dev, void *data, memcpy(audio.status, params->iec.status, min(sizeof(audio.status), sizeof(params->iec.status)));
+ /* Compatibility */ + switch (params->sample_width) { + case 16: + audio.bclk_ratio = 32; + break; + case 18: + case 20: + case 24: + audio.bclk_ratio = 48; + break; + default: + case 32: + audio.bclk_ratio = 64; + break; + } + switch (daifmt->fmt) { case HDMI_I2S: audio.format = AFMT_I2S | AFMT_I2S_PHILIPS; diff --git a/include/drm/i2c/tda998x.h b/include/drm/i2c/tda998x.h index b0864f0be017..4e0f0cd2d428 100644 --- a/include/drm/i2c/tda998x.h +++ b/include/drm/i2c/tda998x.h @@ -19,6 +19,7 @@ enum { struct tda998x_audio_params { u8 config; u8 format; + u8 bclk_ratio; unsigned sample_width; unsigned sample_rate; struct hdmi_audio_infoframe cea;