From 2e9943aab5680ec7bce07f4b980e5e2a86ff294a Mon Sep 17 00:00:00 2001 From: Russell King Date: Fri, 1 Mar 2019 14:14:14 +0000 Subject: [PATCH 01/13] drm/i2c: tda998x: introduce tda998x_audio_settings Introduce a structure to hold the register values to be programmed while programming the TDA998x audio settings. This is currently a stub structure, which will be populated in subsequent commits. When we initialise this from the platform data, only do so if there is a valid audio format specification. Tested-by: Sven Van Asbroeck Signed-off-by: Russell King --- drivers/gpu/drm/i2c/tda998x_drv.c | 68 +++++++++++++++++-------------- 1 file changed, 37 insertions(+), 31 deletions(-) diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c index 7f34601bb515..0668fb3537f2 100644 --- a/drivers/gpu/drm/i2c/tda998x_drv.c +++ b/drivers/gpu/drm/i2c/tda998x_drv.c @@ -40,6 +40,10 @@ struct tda998x_audio_port { u8 config; /* AP value */ }; +struct tda998x_audio_settings { + struct tda998x_audio_params params; +}; + struct tda998x_priv { struct i2c_client *cec; struct i2c_client *hdmi; @@ -54,7 +58,7 @@ struct tda998x_priv { u8 vip_cntrl_1; u8 vip_cntrl_2; unsigned long tmds_clock; - struct tda998x_audio_params audio_params; + struct tda998x_audio_settings audio; struct platform_device *audio_pdev; struct mutex audio_mutex; @@ -833,7 +837,7 @@ tda998x_write_if(struct tda998x_priv *priv, u8 bit, u16 addr, } static int tda998x_write_aif(struct tda998x_priv *priv, - struct hdmi_audio_infoframe *cea) + const struct hdmi_audio_infoframe *cea) { union hdmi_infoframe frame; @@ -869,18 +873,17 @@ static void tda998x_audio_mute(struct tda998x_priv *priv, bool on) } } -static int -tda998x_configure_audio(struct tda998x_priv *priv, - struct tda998x_audio_params *params) +static int tda998x_configure_audio(struct tda998x_priv *priv, + const struct tda998x_audio_settings *settings) { u8 buf[6], clksel_aip, clksel_fs, cts_n, adiv; u32 n; /* Enable audio ports */ - reg_write(priv, REG_ENA_AP, params->config); + reg_write(priv, REG_ENA_AP, settings->params.config); /* Set audio input source */ - switch (params->format) { + switch (settings->params.format) { case AFMT_SPDIF: reg_write(priv, REG_ENA_ACLK, 0); reg_write(priv, REG_MUX_AP, MUX_AP_SELECT_SPDIF); @@ -894,7 +897,7 @@ 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 (settings->params.sample_width) { case 16: cts_n = CTS_N_M(3) | CTS_N_K(1); break; @@ -932,7 +935,7 @@ tda998x_configure_audio(struct tda998x_priv *priv, adiv++; /* AUDIO_DIV_SERCLK_16 */ /* S/PDIF asks for a larger divider */ - if (params->format == AFMT_SPDIF) + if (settings->params.format == AFMT_SPDIF) adiv++; /* AUDIO_DIV_SERCLK_16 or _32 */ reg_write(priv, REG_AUDIO_DIV, adiv); @@ -941,7 +944,7 @@ tda998x_configure_audio(struct tda998x_priv *priv, * This is the approximate value of N, which happens to be * the recommended values for non-coherent clocks. */ - n = 128 * params->sample_rate / 1000; + n = 128 * settings->params.sample_rate / 1000; /* Write the CTS and N values */ buf[0] = 0x44; @@ -963,17 +966,17 @@ tda998x_configure_audio(struct tda998x_priv *priv, * The REG_CH_STAT_B-registers skip IEC958 AES2 byte, because * there is a separate register for each I2S wire. */ - buf[0] = params->status[0]; - buf[1] = params->status[1]; - buf[2] = params->status[3]; - buf[3] = params->status[4]; + buf[0] = settings->params.status[0]; + buf[1] = settings->params.status[1]; + buf[2] = settings->params.status[3]; + buf[3] = settings->params.status[4]; reg_write_range(priv, REG_CH_STAT_B(0), buf, 4); tda998x_audio_mute(priv, true); msleep(20); tda998x_audio_mute(priv, false); - return tda998x_write_aif(priv, ¶ms->cea); + return tda998x_write_aif(priv, &settings->params.cea); } static int tda998x_audio_hw_params(struct device *dev, void *data, @@ -982,14 +985,16 @@ static int tda998x_audio_hw_params(struct device *dev, void *data, { struct tda998x_priv *priv = dev_get_drvdata(dev); int i, ret; - struct tda998x_audio_params audio = { - .sample_width = params->sample_width, - .sample_rate = params->sample_rate, - .cea = params->cea, + struct tda998x_audio_settings audio = { + .params = { + .sample_width = params->sample_width, + .sample_rate = params->sample_rate, + .cea = params->cea, + }, }; - memcpy(audio.status, params->iec.status, - min(sizeof(audio.status), sizeof(params->iec.status))); + memcpy(audio.params.status, params->iec.status, + min(sizeof(audio.params.status), sizeof(params->iec.status))); switch (daifmt->fmt) { case HDMI_I2S: @@ -1003,21 +1008,21 @@ static int tda998x_audio_hw_params(struct device *dev, void *data, } for (i = 0; i < ARRAY_SIZE(priv->audio_port); i++) if (priv->audio_port[i].format == AFMT_I2S) - audio.config = priv->audio_port[i].config; - audio.format = AFMT_I2S; + audio.params.config = priv->audio_port[i].config; + audio.params.format = AFMT_I2S; break; case HDMI_SPDIF: for (i = 0; i < ARRAY_SIZE(priv->audio_port); i++) if (priv->audio_port[i].format == AFMT_SPDIF) - audio.config = priv->audio_port[i].config; - audio.format = AFMT_SPDIF; + audio.params.config = priv->audio_port[i].config; + audio.params.format = AFMT_SPDIF; break; default: dev_err(dev, "%s: Invalid format %d\n", __func__, daifmt->fmt); return -EINVAL; } - if (audio.config == 0) { + if (audio.params.config == 0) { dev_err(dev, "%s: No audio configuration found\n", __func__); return -EINVAL; } @@ -1029,7 +1034,7 @@ static int tda998x_audio_hw_params(struct device *dev, void *data, ret = 0; if (ret == 0) - priv->audio_params = audio; + priv->audio = audio; mutex_unlock(&priv->audio_mutex); return ret; @@ -1043,7 +1048,7 @@ static void tda998x_audio_shutdown(struct device *dev, void *data) reg_write(priv, REG_ENA_AP, 0); - priv->audio_params.format = AFMT_UNUSED; + priv->audio.params.format = AFMT_UNUSED; mutex_unlock(&priv->audio_mutex); } @@ -1549,9 +1554,9 @@ static void tda998x_bridge_mode_set(struct drm_bridge *bridge, tda998x_write_avi(priv, adjusted_mode); - if (priv->audio_params.format != AFMT_UNUSED && + if (priv->audio.params.format != AFMT_UNUSED && priv->sink_has_audio) - tda998x_configure_audio(priv, &priv->audio_params); + tda998x_configure_audio(priv, &priv->audio); } mutex_unlock(&priv->audio_mutex); @@ -1626,7 +1631,8 @@ static void tda998x_set_config(struct tda998x_priv *priv, VIP_CNTRL_2_SWAP_F(p->swap_f) | (p->mirr_f ? VIP_CNTRL_2_MIRR_F : 0); - priv->audio_params = p->audio_params; + if (p->audio_params.format != AFMT_UNUSED) + priv->audio.params = p->audio_params; } static void tda998x_destroy(struct device *dev) From 935b9ca357443250fded91fad0e973555528b390 Mon Sep 17 00:00:00 2001 From: Russell King Date: Fri, 22 Feb 2019 16:14:36 +0000 Subject: [PATCH 02/13] drm/i2c: tda998x: implement different I2S flavours Add support for the left and right justified I2S formats as well as the more tranditional "Philips" I2S format. Tested-by: Peter Ujfalusi Tested-by: Sven Van Asbroeck Signed-off-by: Russell King --- drivers/gpu/drm/i2c/tda998x_drv.c | 51 ++++++++++++++++++++----------- 1 file changed, 33 insertions(+), 18 deletions(-) diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c index 0668fb3537f2..0592fa48e69e 100644 --- a/drivers/gpu/drm/i2c/tda998x_drv.c +++ b/drivers/gpu/drm/i2c/tda998x_drv.c @@ -42,6 +42,7 @@ struct tda998x_audio_port { struct tda998x_audio_settings { struct tda998x_audio_params params; + u8 i2s_format; }; struct tda998x_priv { @@ -246,7 +247,9 @@ struct tda998x_priv { # define HVF_CNTRL_1_SEMI_PLANAR (1 << 6) #define REG_RPT_CNTRL REG(0x00, 0xf0) /* write */ #define REG_I2S_FORMAT REG(0x00, 0xfc) /* read/write */ -# define I2S_FORMAT(x) (((x) & 3) << 0) +# define I2S_FORMAT_PHILIPS (0 << 0) +# define I2S_FORMAT_LEFT_J (2 << 0) +# define I2S_FORMAT_RIGHT_J (3 << 0) #define REG_AIP_CLKSEL REG(0x00, 0xfd) /* write */ # define AIP_CLKSEL_AIP_SPDIF (0 << 3) # define AIP_CLKSEL_AIP_I2S (1 << 3) @@ -918,6 +921,7 @@ static int tda998x_configure_audio(struct tda998x_priv *priv, return -EINVAL; } + reg_write(priv, REG_I2S_FORMAT, settings->i2s_format); reg_write(priv, REG_AIP_CLKSEL, clksel_aip); reg_clear(priv, REG_AIP_CNTRL_0, AIP_CNTRL_0_LAYOUT | AIP_CNTRL_0_ACR_MAN); /* auto CTS */ @@ -984,6 +988,7 @@ static int tda998x_audio_hw_params(struct device *dev, void *data, struct hdmi_codec_params *params) { struct tda998x_priv *priv = dev_get_drvdata(dev); + bool spdif = daifmt->fmt == HDMI_SPDIF; int i, ret; struct tda998x_audio_settings audio = { .params = { @@ -998,35 +1003,43 @@ static int tda998x_audio_hw_params(struct device *dev, void *data, switch (daifmt->fmt) { case HDMI_I2S: - if (daifmt->bit_clk_inv || daifmt->frame_clk_inv || - daifmt->bit_clk_master || daifmt->frame_clk_master) { - dev_err(dev, "%s: Bad flags %d %d %d %d\n", __func__, - daifmt->bit_clk_inv, daifmt->frame_clk_inv, - daifmt->bit_clk_master, - daifmt->frame_clk_master); - return -EINVAL; - } - for (i = 0; i < ARRAY_SIZE(priv->audio_port); i++) - if (priv->audio_port[i].format == AFMT_I2S) - audio.params.config = priv->audio_port[i].config; - audio.params.format = AFMT_I2S; + audio.i2s_format = I2S_FORMAT_PHILIPS; + break; + case HDMI_LEFT_J: + audio.i2s_format = I2S_FORMAT_LEFT_J; + break; + case HDMI_RIGHT_J: + audio.i2s_format = I2S_FORMAT_RIGHT_J; break; case HDMI_SPDIF: - for (i = 0; i < ARRAY_SIZE(priv->audio_port); i++) - if (priv->audio_port[i].format == AFMT_SPDIF) - audio.params.config = priv->audio_port[i].config; - audio.params.format = AFMT_SPDIF; + audio.i2s_format = 0; break; default: dev_err(dev, "%s: Invalid format %d\n", __func__, daifmt->fmt); return -EINVAL; } + audio.params.format = spdif ? AFMT_SPDIF : AFMT_I2S; + + for (i = 0; i < ARRAY_SIZE(priv->audio_port); i++) + if (priv->audio_port[i].format == audio.params.format) + audio.params.config = priv->audio_port[i].config; + if (audio.params.config == 0) { dev_err(dev, "%s: No audio configuration found\n", __func__); return -EINVAL; } + if (!spdif && + (daifmt->bit_clk_inv || daifmt->frame_clk_inv || + daifmt->bit_clk_master || daifmt->frame_clk_master)) { + dev_err(dev, "%s: Bad flags %d %d %d %d\n", __func__, + daifmt->bit_clk_inv, daifmt->frame_clk_inv, + daifmt->bit_clk_master, + daifmt->frame_clk_master); + return -EINVAL; + } + mutex_lock(&priv->audio_mutex); if (priv->supports_infoframes && priv->sink_has_audio) ret = tda998x_configure_audio(priv, &audio); @@ -1631,8 +1644,10 @@ static void tda998x_set_config(struct tda998x_priv *priv, VIP_CNTRL_2_SWAP_F(p->swap_f) | (p->mirr_f ? VIP_CNTRL_2_MIRR_F : 0); - if (p->audio_params.format != AFMT_UNUSED) + if (p->audio_params.format != AFMT_UNUSED) { priv->audio.params = p->audio_params; + priv->audio.i2s_format = I2S_FORMAT_PHILIPS; + } } static void tda998x_destroy(struct device *dev) From 7dad3740aeb7103817e38a191810dbb81afd692e Mon Sep 17 00:00:00 2001 From: Russell King Date: Wed, 27 Feb 2019 00:04:53 +0000 Subject: [PATCH 03/13] drm/i2c: tda998x: improve programming of audio divisor Improve the selection of the audio clock divisor so that more modes and sample rates work. Tested-by: Sven Van Asbroeck Signed-off-by: Russell King --- drivers/gpu/drm/i2c/tda998x_drv.c | 44 ++++++++++++++++++++----------- 1 file changed, 28 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c index 0592fa48e69e..f23aee65846c 100644 --- a/drivers/gpu/drm/i2c/tda998x_drv.c +++ b/drivers/gpu/drm/i2c/tda998x_drv.c @@ -865,6 +865,32 @@ tda998x_write_avi(struct tda998x_priv *priv, const struct drm_display_mode *mode /* Audio support */ +/* + * The audio clock divisor register controls a divider producing Audio_Clk_Out + * from SERclk by dividing it by 2^n where 0 <= n <= 5. We don't know what + * Audio_Clk_Out or SERclk are. We guess SERclk is the same as TMDS clock. + * + * It seems that Audio_Clk_Out must be the smallest value that is greater + * than 128*fs, otherwise audio does not function. There is some suggestion + * that 126*fs is a better value. + */ +static u8 tda998x_get_adiv(struct tda998x_priv *priv, unsigned int fs) +{ + unsigned long min_audio_clk = fs * 128; + unsigned long ser_clk = priv->tmds_clock * 1000; + u8 adiv; + + for (adiv = AUDIO_DIV_SERCLK_32; adiv != AUDIO_DIV_SERCLK_1; adiv--) + if (ser_clk > min_audio_clk << adiv) + break; + + dev_dbg(&priv->hdmi->dev, + "ser_clk=%luHz fs=%uHz min_aclk=%luHz adiv=%d\n", + ser_clk, fs, min_audio_clk, adiv); + + return adiv; +} + static void tda998x_audio_mute(struct tda998x_priv *priv, bool on) { if (on) { @@ -882,6 +908,8 @@ static int tda998x_configure_audio(struct tda998x_priv *priv, u8 buf[6], clksel_aip, clksel_fs, cts_n, adiv; u32 n; + adiv = tda998x_get_adiv(priv, settings->params.sample_rate); + /* Enable audio ports */ reg_write(priv, REG_ENA_AP, settings->params.config); @@ -926,22 +954,6 @@ static int 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); - - /* - * Audio input somehow depends on HDMI line rate which is - * related to pixclk. Testing showed that modes with pixclk - * >100MHz need a larger divider while <40MHz need the default. - * There is no detailed info in the datasheet, so we just - * assume 100MHz requires larger divider. - */ - adiv = AUDIO_DIV_SERCLK_8; - if (priv->tmds_clock > 100000) - adiv++; /* AUDIO_DIV_SERCLK_16 */ - - /* S/PDIF asks for a larger divider */ - if (settings->params.format == AFMT_SPDIF) - adiv++; /* AUDIO_DIV_SERCLK_16 or _32 */ - reg_write(priv, REG_AUDIO_DIV, adiv); /* From a03a915b8387286dfd1e7500705124414802ede7 Mon Sep 17 00:00:00 2001 From: Russell King Date: Fri, 22 Feb 2019 20:53:38 +0000 Subject: [PATCH 04/13] drm/i2c: tda998x: derive CTS_N value from aclk sample rate ratio MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The TDA998x derives the CTS value using the supplied I2S bit clock (ACLK, in TDA998x parlence) rather than 128·fs. TDA998x uses two constants named m and k in the CTS generator such that we have this relationship between the I2S source ACLK and the sink fs: 128·fs_sink = ACLK·m / k Where ACLK = aclk_ratio·fs_source. When audio support was originally added, we supported a fixed ratio of 64·fs, intending to support the Kirkwood I2S on Dove. However, when hdmi-codec support was added, this was changed to scale the ratio with the sample width, which would've broken its use with Kirkwood I2S. We are now starting to see other users whose I2S blocks send at 64·fs for 16-bit samples, so we need to reinstate the support for the fixed ratio I2S bit clock. This commit takes a step towards supporting these configurations by selecting the CTS_N register m and k values based on the bit clock ratio. However, as the driver is not given the bit clock ratio from ALSA, continue deriving this from the sample width. This will be addressed in a later commit. Tested-by: Sven Van Asbroeck Signed-off-by: Russell King --- drivers/gpu/drm/i2c/tda998x_drv.c | 94 ++++++++++++++++++++++++------- 1 file changed, 74 insertions(+), 20 deletions(-) diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c index f23aee65846c..9b3e47f408ca 100644 --- a/drivers/gpu/drm/i2c/tda998x_drv.c +++ b/drivers/gpu/drm/i2c/tda998x_drv.c @@ -43,6 +43,7 @@ struct tda998x_audio_port { struct tda998x_audio_settings { struct tda998x_audio_params params; u8 i2s_format; + u8 cts_n; }; struct tda998x_priv { @@ -891,6 +892,58 @@ static u8 tda998x_get_adiv(struct tda998x_priv *priv, unsigned int fs) return adiv; } +/* + * In auto-CTS mode, the TDA998x uses a "measured time stamp" counter to + * generate the CTS value. It appears that the "measured time stamp" is + * the number of TDMS clock cycles within a number of audio input clock + * cycles defined by the k and N parameters defined below, in a similar + * way to that which is set out in the CTS generation in the HDMI spec. + * + * tmdsclk ----> mts -> /m ---> CTS + * ^ + * sclk -> /k -> /N + * + * CTS = mts / m, where m is 2^M. + * /k is a divider based on the K value below, K+1 for K < 4, or 8 for K >= 4 + * /N is a divider based on the HDMI specified N value. + * + * This produces the following equation: + * CTS = tmds_clock * k * N / (sclk * m) + * + * When combined with the sink-side equation, and realising that sclk is + * bclk_ratio * fs, we end up with: + * k = m * bclk_ratio / 128. + * + * Note: S/PDIF always uses a bclk_ratio of 64. + */ +static int tda998x_derive_cts_n(struct tda998x_priv *priv, + struct tda998x_audio_settings *settings, + unsigned int ratio) +{ + switch (ratio) { + case 16: + settings->cts_n = CTS_N_M(3) | CTS_N_K(0); + break; + case 32: + settings->cts_n = CTS_N_M(3) | CTS_N_K(1); + break; + case 48: + settings->cts_n = CTS_N_M(3) | CTS_N_K(2); + break; + case 64: + settings->cts_n = CTS_N_M(3) | CTS_N_K(3); + break; + case 128: + settings->cts_n = CTS_N_M(0) | CTS_N_K(0); + break; + default: + dev_err(&priv->hdmi->dev, "unsupported bclk ratio %ufs\n", + ratio); + return -EINVAL; + } + return 0; +} + static void tda998x_audio_mute(struct tda998x_priv *priv, bool on) { if (on) { @@ -905,7 +958,7 @@ static void tda998x_audio_mute(struct tda998x_priv *priv, bool on) static int tda998x_configure_audio(struct tda998x_priv *priv, const struct tda998x_audio_settings *settings) { - u8 buf[6], clksel_aip, clksel_fs, cts_n, adiv; + u8 buf[6], clksel_aip, clksel_fs, adiv; u32 n; adiv = tda998x_get_adiv(priv, settings->params.sample_rate); @@ -920,7 +973,6 @@ static int tda998x_configure_audio(struct tda998x_priv *priv, reg_write(priv, REG_MUX_AP, MUX_AP_SELECT_SPDIF); clksel_aip = AIP_CLKSEL_AIP_SPDIF; clksel_fs = AIP_CLKSEL_FS_FS64SPDIF; - cts_n = CTS_N_M(3) | CTS_N_K(3); break; case AFMT_I2S: @@ -928,20 +980,6 @@ static int 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 (settings->params.sample_width) { - case 16: - cts_n = CTS_N_M(3) | CTS_N_K(1); - break; - case 18: - case 20: - case 24: - cts_n = CTS_N_M(3) | CTS_N_K(2); - break; - default: - case 32: - cts_n = CTS_N_M(3) | CTS_N_K(3); - break; - } break; default: @@ -953,7 +991,7 @@ static int tda998x_configure_audio(struct tda998x_priv *priv, reg_write(priv, REG_AIP_CLKSEL, clksel_aip); 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_CTS_N, settings->cts_n); reg_write(priv, REG_AUDIO_DIV, adiv); /* @@ -1000,6 +1038,7 @@ static int tda998x_audio_hw_params(struct device *dev, void *data, struct hdmi_codec_params *params) { struct tda998x_priv *priv = dev_get_drvdata(dev); + unsigned int bclk_ratio; bool spdif = daifmt->fmt == HDMI_SPDIF; int i, ret; struct tda998x_audio_settings audio = { @@ -1052,6 +1091,11 @@ static int tda998x_audio_hw_params(struct device *dev, void *data, return -EINVAL; } + bclk_ratio = spdif ? 64 : params->sample_width * 2; + ret = tda998x_derive_cts_n(priv, &audio, bclk_ratio); + if (ret < 0) + return ret; + mutex_lock(&priv->audio_mutex); if (priv->supports_infoframes && priv->sink_has_audio) ret = tda998x_configure_audio(priv, &audio); @@ -1640,8 +1684,8 @@ static int tda998x_get_audio_ports(struct tda998x_priv *priv, return 0; } -static void tda998x_set_config(struct tda998x_priv *priv, - const struct tda998x_encoder_params *p) +static int tda998x_set_config(struct tda998x_priv *priv, + const struct tda998x_encoder_params *p) { priv->vip_cntrl_0 = VIP_CNTRL_0_SWAP_A(p->swap_a) | (p->mirr_a ? VIP_CNTRL_0_MIRR_A : 0) | @@ -1657,9 +1701,17 @@ static void tda998x_set_config(struct tda998x_priv *priv, (p->mirr_f ? VIP_CNTRL_2_MIRR_F : 0); if (p->audio_params.format != AFMT_UNUSED) { + unsigned int ratio; + bool spdif = p->audio_params.format == AFMT_SPDIF; + priv->audio.params = p->audio_params; priv->audio.i2s_format = I2S_FORMAT_PHILIPS; + + ratio = spdif ? 64 : p->audio_params.sample_width * 2; + return tda998x_derive_cts_n(priv, &priv->audio, ratio); } + + return 0; } static void tda998x_destroy(struct device *dev) @@ -1861,7 +1913,9 @@ static int tda998x_create(struct device *dev) if (priv->audio_port[0].format != AFMT_UNUSED) tda998x_audio_codec_init(priv, &client->dev); } else if (dev->platform_data) { - tda998x_set_config(priv, dev->platform_data); + ret = tda998x_set_config(priv, dev->platform_data); + if (ret) + goto fail; } priv->bridge.funcs = &tda998x_bridge_funcs; From 82642ab7345d7e736c6f1dee58c033ea6b41d2e9 Mon Sep 17 00:00:00 2001 From: Russell King Date: Fri, 1 Mar 2019 19:12:28 +0000 Subject: [PATCH 05/13] drm/i2c: tda998x: store audio port enable in settings Store the audio port enable register in the audio settings structure, which can never be zero for a valid audio configuration. Use this to signal whether we have audio configured, rather than AFMT_UNUSED. Tested-by: Sven Van Asbroeck Signed-off-by: Russell King --- drivers/gpu/drm/i2c/tda998x_drv.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c index 9b3e47f408ca..9590c4f92969 100644 --- a/drivers/gpu/drm/i2c/tda998x_drv.c +++ b/drivers/gpu/drm/i2c/tda998x_drv.c @@ -42,6 +42,7 @@ struct tda998x_audio_port { struct tda998x_audio_settings { struct tda998x_audio_params params; + u8 ena_ap; u8 i2s_format; u8 cts_n; }; @@ -961,10 +962,14 @@ static int tda998x_configure_audio(struct tda998x_priv *priv, u8 buf[6], clksel_aip, clksel_fs, adiv; u32 n; + /* If audio is not configured, there is nothing to do. */ + if (settings->ena_ap == 0) + return 0; + adiv = tda998x_get_adiv(priv, settings->params.sample_rate); /* Enable audio ports */ - reg_write(priv, REG_ENA_AP, settings->params.config); + reg_write(priv, REG_ENA_AP, settings->ena_ap); /* Set audio input source */ switch (settings->params.format) { @@ -1074,9 +1079,9 @@ static int tda998x_audio_hw_params(struct device *dev, void *data, for (i = 0; i < ARRAY_SIZE(priv->audio_port); i++) if (priv->audio_port[i].format == audio.params.format) - audio.params.config = priv->audio_port[i].config; + audio.ena_ap = priv->audio_port[i].config; - if (audio.params.config == 0) { + if (audio.ena_ap == 0) { dev_err(dev, "%s: No audio configuration found\n", __func__); return -EINVAL; } @@ -1116,8 +1121,7 @@ static void tda998x_audio_shutdown(struct device *dev, void *data) mutex_lock(&priv->audio_mutex); reg_write(priv, REG_ENA_AP, 0); - - priv->audio.params.format = AFMT_UNUSED; + priv->audio.ena_ap = 0; mutex_unlock(&priv->audio_mutex); } @@ -1623,8 +1627,7 @@ static void tda998x_bridge_mode_set(struct drm_bridge *bridge, tda998x_write_avi(priv, adjusted_mode); - if (priv->audio.params.format != AFMT_UNUSED && - priv->sink_has_audio) + if (priv->sink_has_audio) tda998x_configure_audio(priv, &priv->audio); } @@ -1705,6 +1708,7 @@ static int tda998x_set_config(struct tda998x_priv *priv, bool spdif = p->audio_params.format == AFMT_SPDIF; priv->audio.params = p->audio_params; + priv->audio.ena_ap = p->audio_params.config; priv->audio.i2s_format = I2S_FORMAT_PHILIPS; ratio = spdif ? 64 : p->audio_params.sample_width * 2; From 7168916072b54934fd949e7ae24695080d4388ba Mon Sep 17 00:00:00 2001 From: Russell King Date: Fri, 1 Mar 2019 19:52:23 +0000 Subject: [PATCH 06/13] drm/i2c: tda998x: index audio port enable config by route type Rather than searching an array for the audio format (which we control) implement indexing by route type. This avoids iterating over the array in several locations. Tested-by: Sven Van Asbroeck Signed-off-by: Russell King --- drivers/gpu/drm/i2c/tda998x_drv.c | 57 ++++++++++++++++--------------- 1 file changed, 29 insertions(+), 28 deletions(-) diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c index 9590c4f92969..91b8ad1da923 100644 --- a/drivers/gpu/drm/i2c/tda998x_drv.c +++ b/drivers/gpu/drm/i2c/tda998x_drv.c @@ -35,9 +35,10 @@ #define DBG(fmt, ...) DRM_DEBUG(fmt"\n", ##__VA_ARGS__) -struct tda998x_audio_port { - u8 format; /* AFMT_xxx */ - u8 config; /* AP value */ +enum { + AUDIO_ROUTE_I2S, + AUDIO_ROUTE_SPDIF, + AUDIO_ROUTE_NUM }; struct tda998x_audio_settings { @@ -79,7 +80,7 @@ struct tda998x_priv { struct drm_bridge bridge; struct drm_connector connector; - struct tda998x_audio_port audio_port[2]; + u8 audio_port_enable[AUDIO_ROUTE_NUM]; struct tda9950_glue cec_glue; struct gpio_desc *calib; struct cec_notifier *cec_notify; @@ -1045,7 +1046,7 @@ static int tda998x_audio_hw_params(struct device *dev, void *data, struct tda998x_priv *priv = dev_get_drvdata(dev); unsigned int bclk_ratio; bool spdif = daifmt->fmt == HDMI_SPDIF; - int i, ret; + int ret; struct tda998x_audio_settings audio = { .params = { .sample_width = params->sample_width, @@ -1077,10 +1078,7 @@ static int tda998x_audio_hw_params(struct device *dev, void *data, audio.params.format = spdif ? AFMT_SPDIF : AFMT_I2S; - for (i = 0; i < ARRAY_SIZE(priv->audio_port); i++) - if (priv->audio_port[i].format == audio.params.format) - audio.ena_ap = priv->audio_port[i].config; - + audio.ena_ap = priv->audio_port_enable[AUDIO_ROUTE_I2S + spdif]; if (audio.ena_ap == 0) { dev_err(dev, "%s: No audio configuration found\n", __func__); return -EINVAL; @@ -1165,16 +1163,11 @@ static int tda998x_audio_codec_init(struct tda998x_priv *priv, .ops = &audio_codec_ops, .max_i2s_channels = 2, }; - int i; - for (i = 0; i < ARRAY_SIZE(priv->audio_port); i++) { - if (priv->audio_port[i].format == AFMT_I2S && - priv->audio_port[i].config != 0) - codec_data.i2s = 1; - if (priv->audio_port[i].format == AFMT_SPDIF && - priv->audio_port[i].config != 0) - codec_data.spdif = 1; - } + if (priv->audio_port_enable[AUDIO_ROUTE_I2S]) + codec_data.i2s = 1; + if (priv->audio_port_enable[AUDIO_ROUTE_SPDIF]) + codec_data.spdif = 1; priv->audio_pdev = platform_device_register_data( dev, HDMI_CODEC_DRV_NAME, PLATFORM_DEVID_AUTO, @@ -1657,7 +1650,7 @@ static int tda998x_get_audio_ports(struct tda998x_priv *priv, return 0; size /= sizeof(u32); - if (size > 2 * ARRAY_SIZE(priv->audio_port) || size % 2 != 0) { + if (size > 2 * ARRAY_SIZE(priv->audio_port_enable) || size % 2 != 0) { dev_err(&priv->hdmi->dev, "Bad number of elements in audio-ports dt-property\n"); return -EINVAL; @@ -1666,23 +1659,30 @@ static int tda998x_get_audio_ports(struct tda998x_priv *priv, size /= 2; for (i = 0; i < size; i++) { + unsigned int route; u8 afmt = be32_to_cpup(&port_data[2*i]); u8 ena_ap = be32_to_cpup(&port_data[2*i+1]); - if (afmt != AFMT_SPDIF && afmt != AFMT_I2S) { + switch (afmt) { + case AFMT_I2S: + route = AUDIO_ROUTE_I2S; + break; + case AFMT_SPDIF: + route = AUDIO_ROUTE_SPDIF; + break; + default: dev_err(&priv->hdmi->dev, "Bad audio format %u\n", afmt); return -EINVAL; } - priv->audio_port[i].format = afmt; - priv->audio_port[i].config = ena_ap; - } + if (priv->audio_port_enable[route]) { + dev_err(&priv->hdmi->dev, + "There can only be on I2S port and one SPDIF port\n"); + return -EINVAL; + } - if (priv->audio_port[0].format == priv->audio_port[1].format) { - dev_err(&priv->hdmi->dev, - "There can only be on I2S port and one SPDIF port\n"); - return -EINVAL; + priv->audio_port_enable[route] = ena_ap; } return 0; } @@ -1914,7 +1914,8 @@ static int tda998x_create(struct device *dev) if (ret) goto fail; - if (priv->audio_port[0].format != AFMT_UNUSED) + if (priv->audio_port_enable[AUDIO_ROUTE_I2S] || + priv->audio_port_enable[AUDIO_ROUTE_SPDIF]) tda998x_audio_codec_init(priv, &client->dev); } else if (dev->platform_data) { ret = tda998x_set_config(priv, dev->platform_data); From e4fe96f11ee4de6612862191efef9242243ed364 Mon Sep 17 00:00:00 2001 From: Russell King Date: Fri, 1 Mar 2019 20:11:17 +0000 Subject: [PATCH 07/13] drm/i2c: tda998x: configure both fields of AIP_CLKSEL together We can configure both fields of the AIP_CLKSEL register with a single write, there is no need to delay the setting of the CTS reference. Tested-by: Sven Van Asbroeck Signed-off-by: Russell King --- drivers/gpu/drm/i2c/tda998x_drv.c | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c index 91b8ad1da923..b1a0e41ca3e0 100644 --- a/drivers/gpu/drm/i2c/tda998x_drv.c +++ b/drivers/gpu/drm/i2c/tda998x_drv.c @@ -960,7 +960,7 @@ static void tda998x_audio_mute(struct tda998x_priv *priv, bool on) static int tda998x_configure_audio(struct tda998x_priv *priv, const struct tda998x_audio_settings *settings) { - u8 buf[6], clksel_aip, clksel_fs, adiv; + u8 buf[6], aip_clksel, adiv; u32 n; /* If audio is not configured, there is nothing to do. */ @@ -977,15 +977,13 @@ static int tda998x_configure_audio(struct tda998x_priv *priv, case AFMT_SPDIF: reg_write(priv, REG_ENA_ACLK, 0); reg_write(priv, REG_MUX_AP, MUX_AP_SELECT_SPDIF); - clksel_aip = AIP_CLKSEL_AIP_SPDIF; - clksel_fs = AIP_CLKSEL_FS_FS64SPDIF; + aip_clksel = AIP_CLKSEL_AIP_SPDIF | AIP_CLKSEL_FS_FS64SPDIF; break; case AFMT_I2S: reg_write(priv, REG_ENA_ACLK, 1); reg_write(priv, REG_MUX_AP, MUX_AP_SELECT_I2S); - clksel_aip = AIP_CLKSEL_AIP_I2S; - clksel_fs = AIP_CLKSEL_FS_ACLK; + aip_clksel = AIP_CLKSEL_AIP_I2S | AIP_CLKSEL_FS_ACLK; break; default: @@ -994,7 +992,7 @@ static int tda998x_configure_audio(struct tda998x_priv *priv, } reg_write(priv, REG_I2S_FORMAT, settings->i2s_format); - reg_write(priv, REG_AIP_CLKSEL, clksel_aip); + reg_write(priv, REG_AIP_CLKSEL, aip_clksel); reg_clear(priv, REG_AIP_CNTRL_0, AIP_CNTRL_0_LAYOUT | AIP_CNTRL_0_ACR_MAN); /* auto CTS */ reg_write(priv, REG_CTS_N, settings->cts_n); @@ -1015,9 +1013,6 @@ static int tda998x_configure_audio(struct tda998x_priv *priv, buf[5] = n >> 16; reg_write_range(priv, REG_ACR_CTS_0, buf, 6); - /* Set CTS clock reference */ - reg_write(priv, REG_AIP_CLKSEL, clksel_aip | clksel_fs); - /* Reset CTS generator */ reg_set(priv, REG_AIP_CNTRL_0, AIP_CNTRL_0_RST_CTS); reg_clear(priv, REG_AIP_CNTRL_0, AIP_CNTRL_0_RST_CTS); From 26f7bf1251c70bafe99f73edd222f6126b395b3b Mon Sep 17 00:00:00 2001 From: Russell King Date: Fri, 1 Mar 2019 21:17:04 +0000 Subject: [PATCH 08/13] drm/i2c: tda998x: move audio routing configuration Move the mux and clocking selection out of tda998x_configure_audio() into the parent functions, so we can validate this when parameters are set outside of the audio mutex. Tested-by: Sven Van Asbroeck Signed-off-by: Russell King --- drivers/gpu/drm/i2c/tda998x_drv.c | 78 +++++++++++++++++++------------ 1 file changed, 47 insertions(+), 31 deletions(-) diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c index b1a0e41ca3e0..0d47cd14011d 100644 --- a/drivers/gpu/drm/i2c/tda998x_drv.c +++ b/drivers/gpu/drm/i2c/tda998x_drv.c @@ -41,7 +41,14 @@ enum { AUDIO_ROUTE_NUM }; +struct tda998x_audio_route { + u8 ena_aclk; + u8 mux_ap; + u8 aip_clksel; +}; + struct tda998x_audio_settings { + const struct tda998x_audio_route *route; struct tda998x_audio_params params; u8 ena_ap; u8 i2s_format; @@ -868,6 +875,34 @@ tda998x_write_avi(struct tda998x_priv *priv, const struct drm_display_mode *mode /* Audio support */ +static const struct tda998x_audio_route tda998x_audio_route[AUDIO_ROUTE_NUM] = { + [AUDIO_ROUTE_I2S] = { + .ena_aclk = 1, + .mux_ap = MUX_AP_SELECT_I2S, + .aip_clksel = AIP_CLKSEL_AIP_I2S | AIP_CLKSEL_FS_ACLK, + }, + [AUDIO_ROUTE_SPDIF] = { + .ena_aclk = 0, + .mux_ap = MUX_AP_SELECT_SPDIF, + .aip_clksel = AIP_CLKSEL_AIP_SPDIF | AIP_CLKSEL_FS_FS64SPDIF, + }, +}; + +/* Configure the TDA998x audio data and clock routing. */ +static int tda998x_derive_routing(struct tda998x_priv *priv, + struct tda998x_audio_settings *s, + unsigned int route) +{ + s->route = &tda998x_audio_route[route]; + s->ena_ap = priv->audio_port_enable[route]; + if (s->ena_ap == 0) { + dev_err(&priv->hdmi->dev, "no audio configuration found\n"); + return -EINVAL; + } + + return 0; +} + /* * The audio clock divisor register controls a divider producing Audio_Clk_Out * from SERclk by dividing it by 2^n where 0 <= n <= 5. We don't know what @@ -960,7 +995,7 @@ static void tda998x_audio_mute(struct tda998x_priv *priv, bool on) static int tda998x_configure_audio(struct tda998x_priv *priv, const struct tda998x_audio_settings *settings) { - u8 buf[6], aip_clksel, adiv; + u8 buf[6], adiv; u32 n; /* If audio is not configured, there is nothing to do. */ @@ -971,28 +1006,10 @@ static int tda998x_configure_audio(struct tda998x_priv *priv, /* Enable audio ports */ reg_write(priv, REG_ENA_AP, settings->ena_ap); - - /* Set audio input source */ - switch (settings->params.format) { - case AFMT_SPDIF: - reg_write(priv, REG_ENA_ACLK, 0); - reg_write(priv, REG_MUX_AP, MUX_AP_SELECT_SPDIF); - aip_clksel = AIP_CLKSEL_AIP_SPDIF | AIP_CLKSEL_FS_FS64SPDIF; - break; - - case AFMT_I2S: - reg_write(priv, REG_ENA_ACLK, 1); - reg_write(priv, REG_MUX_AP, MUX_AP_SELECT_I2S); - aip_clksel = AIP_CLKSEL_AIP_I2S | AIP_CLKSEL_FS_ACLK; - break; - - default: - dev_err(&priv->hdmi->dev, "Unsupported I2S format\n"); - return -EINVAL; - } - + reg_write(priv, REG_ENA_ACLK, settings->route->ena_aclk); + reg_write(priv, REG_MUX_AP, settings->route->mux_ap); reg_write(priv, REG_I2S_FORMAT, settings->i2s_format); - reg_write(priv, REG_AIP_CLKSEL, aip_clksel); + reg_write(priv, REG_AIP_CLKSEL, settings->route->aip_clksel); reg_clear(priv, REG_AIP_CNTRL_0, AIP_CNTRL_0_LAYOUT | AIP_CNTRL_0_ACR_MAN); /* auto CTS */ reg_write(priv, REG_CTS_N, settings->cts_n); @@ -1071,14 +1088,6 @@ static int tda998x_audio_hw_params(struct device *dev, void *data, return -EINVAL; } - audio.params.format = spdif ? AFMT_SPDIF : AFMT_I2S; - - audio.ena_ap = priv->audio_port_enable[AUDIO_ROUTE_I2S + spdif]; - if (audio.ena_ap == 0) { - dev_err(dev, "%s: No audio configuration found\n", __func__); - return -EINVAL; - } - if (!spdif && (daifmt->bit_clk_inv || daifmt->frame_clk_inv || daifmt->bit_clk_master || daifmt->frame_clk_master)) { @@ -1089,6 +1098,10 @@ static int tda998x_audio_hw_params(struct device *dev, void *data, return -EINVAL; } + ret = tda998x_derive_routing(priv, &audio, AUDIO_ROUTE_I2S + spdif); + if (ret < 0) + return ret; + bclk_ratio = spdif ? 64 : params->sample_width * 2; ret = tda998x_derive_cts_n(priv, &audio, bclk_ratio); if (ret < 0) @@ -1699,9 +1712,12 @@ static int tda998x_set_config(struct tda998x_priv *priv, (p->mirr_f ? VIP_CNTRL_2_MIRR_F : 0); if (p->audio_params.format != AFMT_UNUSED) { - unsigned int ratio; + unsigned int ratio, route; bool spdif = p->audio_params.format == AFMT_SPDIF; + route = AUDIO_ROUTE_I2S + spdif; + + priv->audio.route = &tda998x_audio_route[route]; priv->audio.params = p->audio_params; priv->audio.ena_ap = p->audio_params.config; priv->audio.i2s_format = I2S_FORMAT_PHILIPS; From 900b2b7250b8fe49270e9272a1d937fa69350538 Mon Sep 17 00:00:00 2001 From: Russell King Date: Fri, 1 Mar 2019 21:21:38 +0000 Subject: [PATCH 09/13] drm/i2c: tda998x: clean up tda998x_configure_audio() tda998x_configure_audio() is called via some paths where an error return is meaningless, and as a result of moving the audio routing code, this function no longer returns any errors, so let's make it void. We can also make tda998x_write_aif() return void as well. tda998x_configure_audio() also only ever needs to write the current audio settings, so simplify the code in tda998x_audio_hw_params() so that can happen. Tested-by: Sven Van Asbroeck Signed-off-by: Russell King --- drivers/gpu/drm/i2c/tda998x_drv.c | 26 ++++++++++---------------- 1 file changed, 10 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c index 0d47cd14011d..e4f0f5699d65 100644 --- a/drivers/gpu/drm/i2c/tda998x_drv.c +++ b/drivers/gpu/drm/i2c/tda998x_drv.c @@ -849,16 +849,14 @@ tda998x_write_if(struct tda998x_priv *priv, u8 bit, u16 addr, reg_set(priv, REG_DIP_IF_FLAGS, bit); } -static int tda998x_write_aif(struct tda998x_priv *priv, - const struct hdmi_audio_infoframe *cea) +static void tda998x_write_aif(struct tda998x_priv *priv, + const struct hdmi_audio_infoframe *cea) { union hdmi_infoframe frame; frame.audio = *cea; tda998x_write_if(priv, DIP_IF_FLAGS_IF4, REG_IF4_HB0, &frame); - - return 0; } static void @@ -992,15 +990,15 @@ static void tda998x_audio_mute(struct tda998x_priv *priv, bool on) } } -static int tda998x_configure_audio(struct tda998x_priv *priv, - const struct tda998x_audio_settings *settings) +static void tda998x_configure_audio(struct tda998x_priv *priv) { + const struct tda998x_audio_settings *settings = &priv->audio; u8 buf[6], adiv; u32 n; /* If audio is not configured, there is nothing to do. */ if (settings->ena_ap == 0) - return 0; + return; adiv = tda998x_get_adiv(priv, settings->params.sample_rate); @@ -1048,7 +1046,7 @@ static int tda998x_configure_audio(struct tda998x_priv *priv, msleep(20); tda998x_audio_mute(priv, false); - return tda998x_write_aif(priv, &settings->params.cea); + tda998x_write_aif(priv, &settings->params.cea); } static int tda998x_audio_hw_params(struct device *dev, void *data, @@ -1108,16 +1106,12 @@ static int tda998x_audio_hw_params(struct device *dev, void *data, return ret; mutex_lock(&priv->audio_mutex); + priv->audio = audio; if (priv->supports_infoframes && priv->sink_has_audio) - ret = tda998x_configure_audio(priv, &audio); - else - ret = 0; - - if (ret == 0) - priv->audio = audio; + tda998x_configure_audio(priv); mutex_unlock(&priv->audio_mutex); - return ret; + return 0; } static void tda998x_audio_shutdown(struct device *dev, void *data) @@ -1629,7 +1623,7 @@ static void tda998x_bridge_mode_set(struct drm_bridge *bridge, tda998x_write_avi(priv, adjusted_mode); if (priv->sink_has_audio) - tda998x_configure_audio(priv, &priv->audio); + tda998x_configure_audio(priv); } mutex_unlock(&priv->audio_mutex); From 125a4f9394c769bc3e4306ff6cc73529bc1fa6dd Mon Sep 17 00:00:00 2001 From: Russell King Date: Fri, 1 Mar 2019 11:32:15 +0000 Subject: [PATCH 10/13] drm/i2c: tda998x: get rid of params in audio settings Get rid of the tda998x_audio_params structure in audio_settings, which is now just used for platform data. Tested-by: Sven Van Asbroeck Signed-off-by: Russell King --- drivers/gpu/drm/i2c/tda998x_drv.c | 43 +++++++++++++++++++------------ 1 file changed, 26 insertions(+), 17 deletions(-) diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c index e4f0f5699d65..384e07696101 100644 --- a/drivers/gpu/drm/i2c/tda998x_drv.c +++ b/drivers/gpu/drm/i2c/tda998x_drv.c @@ -49,7 +49,9 @@ struct tda998x_audio_route { struct tda998x_audio_settings { const struct tda998x_audio_route *route; - struct tda998x_audio_params params; + struct hdmi_audio_infoframe cea; + unsigned int sample_rate; + u8 status[5]; u8 ena_ap; u8 i2s_format; u8 cts_n; @@ -1000,7 +1002,7 @@ static void tda998x_configure_audio(struct tda998x_priv *priv) if (settings->ena_ap == 0) return; - adiv = tda998x_get_adiv(priv, settings->params.sample_rate); + adiv = tda998x_get_adiv(priv, settings->sample_rate); /* Enable audio ports */ reg_write(priv, REG_ENA_AP, settings->ena_ap); @@ -1017,7 +1019,7 @@ static void tda998x_configure_audio(struct tda998x_priv *priv) * This is the approximate value of N, which happens to be * the recommended values for non-coherent clocks. */ - n = 128 * settings->params.sample_rate / 1000; + n = 128 * settings->sample_rate / 1000; /* Write the CTS and N values */ buf[0] = 0x44; @@ -1036,17 +1038,17 @@ static void tda998x_configure_audio(struct tda998x_priv *priv) * The REG_CH_STAT_B-registers skip IEC958 AES2 byte, because * there is a separate register for each I2S wire. */ - buf[0] = settings->params.status[0]; - buf[1] = settings->params.status[1]; - buf[2] = settings->params.status[3]; - buf[3] = settings->params.status[4]; + buf[0] = settings->status[0]; + buf[1] = settings->status[1]; + buf[2] = settings->status[3]; + buf[3] = settings->status[4]; reg_write_range(priv, REG_CH_STAT_B(0), buf, 4); tda998x_audio_mute(priv, true); msleep(20); tda998x_audio_mute(priv, false); - tda998x_write_aif(priv, &settings->params.cea); + tda998x_write_aif(priv, &settings->cea); } static int tda998x_audio_hw_params(struct device *dev, void *data, @@ -1058,15 +1060,12 @@ static int tda998x_audio_hw_params(struct device *dev, void *data, bool spdif = daifmt->fmt == HDMI_SPDIF; int ret; struct tda998x_audio_settings audio = { - .params = { - .sample_width = params->sample_width, - .sample_rate = params->sample_rate, - .cea = params->cea, - }, + .sample_rate = params->sample_rate, + .cea = params->cea, }; - memcpy(audio.params.status, params->iec.status, - min(sizeof(audio.params.status), sizeof(params->iec.status))); + memcpy(audio.status, params->iec.status, + min(sizeof(audio.status), sizeof(params->iec.status))); switch (daifmt->fmt) { case HDMI_I2S: @@ -1678,9 +1677,15 @@ static int tda998x_get_audio_ports(struct tda998x_priv *priv, return -EINVAL; } + if (!ena_ap) { + dev_err(&priv->hdmi->dev, "invalid zero port config\n"); + continue; + } + if (priv->audio_port_enable[route]) { dev_err(&priv->hdmi->dev, - "There can only be on I2S port and one SPDIF port\n"); + "%s format already configured\n", + route == AUDIO_ROUTE_SPDIF ? "SPDIF" : "I2S"); return -EINVAL; } @@ -1712,7 +1717,11 @@ static int tda998x_set_config(struct tda998x_priv *priv, route = AUDIO_ROUTE_I2S + spdif; priv->audio.route = &tda998x_audio_route[route]; - priv->audio.params = p->audio_params; + priv->audio.cea = p->audio_params.cea; + priv->audio.sample_rate = p->audio_params.sample_rate; + memcpy(priv->audio.status, p->audio_params.status, + min(sizeof(priv->audio.status), + sizeof(p->audio_params.status))); priv->audio.ena_ap = p->audio_params.config; priv->audio.i2s_format = I2S_FORMAT_PHILIPS; From 2807ba75970367c528a9c43aef6296c95eade5be Mon Sep 17 00:00:00 2001 From: Russell King Date: Sun, 8 Jul 2018 22:19:02 +0100 Subject: [PATCH 11/13] drm/i2c: tda998x: add support for pixel repeated modes TDA998x has no support for pixel repeated modes, and the code notes this as a "TODO" item. The implementation appears to be relatively simple, so lets add it. We need to calculate the serializer clock divisor based on the TMDS clock rate, set the repeat control, and set the serializer pixel repeat count. Since the audio code needs the actual TMDS clock, record that. Tested-by: Sven Van Asbroeck Signed-off-by: Russell King --- drivers/gpu/drm/i2c/tda998x_drv.c | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c index 384e07696101..d4409aa5ed7a 100644 --- a/drivers/gpu/drm/i2c/tda998x_drv.c +++ b/drivers/gpu/drm/i2c/tda998x_drv.c @@ -258,6 +258,7 @@ struct tda998x_priv { # define HVF_CNTRL_1_PAD(x) (((x) & 3) << 4) # define HVF_CNTRL_1_SEMI_PLANAR (1 << 6) #define REG_RPT_CNTRL REG(0x00, 0xf0) /* write */ +# define RPT_CNTRL_REPEAT(x) ((x) & 15) #define REG_I2S_FORMAT REG(0x00, 0xfc) /* read/write */ # define I2S_FORMAT_PHILIPS (0 << 0) # define I2S_FORMAT_LEFT_J (2 << 0) @@ -1423,7 +1424,7 @@ static void tda998x_bridge_mode_set(struct drm_bridge *bridge, u16 vwin1_line_s, vwin1_line_e; u16 vwin2_line_s, vwin2_line_e; u16 de_pix_s, de_pix_e; - u8 reg, div, rep; + u8 reg, div, rep, sel_clk; /* * Internally TDA998x is using ITU-R BT.656 style sync but @@ -1486,7 +1487,16 @@ static void tda998x_bridge_mode_set(struct drm_bridge *bridge, (mode->vsync_end - mode->vsync_start)/2; } - tmds_clock = mode->clock; + /* + * Select pixel repeat depending on the double-clock flag + * (which means we have to repeat each pixel once.) + */ + rep = mode->flags & DRM_MODE_FLAG_DBLCLK ? 1 : 0; + sel_clk = SEL_CLK_ENA_SC_CLK | SEL_CLK_SEL_CLK1 | + SEL_CLK_SEL_VRF_CLK(rep ? 2 : 0); + + /* the TMDS clock is scaled up by the pixel repeat */ + tmds_clock = mode->clock * (1 + rep); /* * The divisor is power-of-2. The TDA9983B datasheet gives @@ -1502,6 +1512,8 @@ static void tda998x_bridge_mode_set(struct drm_bridge *bridge, mutex_lock(&priv->audio_mutex); + priv->tmds_clock = tmds_clock; + /* mute the audio FIFO: */ reg_set(priv, REG_AIP_CNTRL_0, AIP_CNTRL_0_RST_FIFO); @@ -1524,12 +1536,8 @@ static void tda998x_bridge_mode_set(struct drm_bridge *bridge, reg_write(priv, REG_SERIALIZER, 0); reg_write(priv, REG_HVF_CNTRL_1, HVF_CNTRL_1_VQR(0)); - /* TODO enable pixel repeat for pixel rates less than 25Msamp/s */ - rep = 0; - reg_write(priv, REG_RPT_CNTRL, 0); - reg_write(priv, REG_SEL_CLK, SEL_CLK_SEL_VRF_CLK(0) | - SEL_CLK_SEL_CLK1 | SEL_CLK_ENA_SC_CLK); - + reg_write(priv, REG_RPT_CNTRL, RPT_CNTRL_REPEAT(rep)); + reg_write(priv, REG_SEL_CLK, sel_clk); reg_write(priv, REG_PLL_SERIAL_2, PLL_SERIAL_2_SRL_NOSC(div) | PLL_SERIAL_2_SRL_PR(rep)); @@ -1597,8 +1605,6 @@ static void tda998x_bridge_mode_set(struct drm_bridge *bridge, /* must be last register set: */ reg_write(priv, REG_TBG_CNTRL_0, 0); - priv->tmds_clock = adjusted_mode->clock; - /* CEA-861B section 6 says that: * CEA version 1 (CEA-861) has no support for infoframes. * CEA version 2 (CEA-861A) supports version 1 AVI infoframes, From fcc22c5f9ddaa8d1bb051878cc7e5f928b9973d8 Mon Sep 17 00:00:00 2001 From: Russell King Date: Tue, 31 Jul 2018 11:12:27 +0100 Subject: [PATCH 12/13] drm/i2c: tda998x: improve correctness of quantisation range CEA-861 says: "A Source shall not send a non-zero Q value that does not correspond to the default RGB Quantization Range for the transmitted Picture unless the Sink indicates support for the Q bit in a Video Capabilities Data Block." Make TDA998x compliant by using the helper to set the quantisation range in the infoframe, and using the TDA998x's colour scaling to appropriately adjust the RGB values sent to the monitor. This ensures that monitors that do not support the Q bit are sent RGB values that are within the expected range. Monitors with support for the Q bit will be sent full-range RGB. Reviewed-by: Brian Starkey Tested-by: Sven Van Asbroeck Signed-off-by: Russell King --- drivers/gpu/drm/i2c/tda998x_drv.c | 36 +++++++++++++++++++++++++++---- 1 file changed, 32 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c index d4409aa5ed7a..2d69aef556a5 100644 --- a/drivers/gpu/drm/i2c/tda998x_drv.c +++ b/drivers/gpu/drm/i2c/tda998x_drv.c @@ -67,6 +67,7 @@ struct tda998x_priv { bool is_on; bool supports_infoframes; bool sink_has_audio; + enum hdmi_quantization_range rgb_quant_range; u8 vip_cntrl_0; u8 vip_cntrl_1; u8 vip_cntrl_2; @@ -870,6 +871,8 @@ tda998x_write_avi(struct tda998x_priv *priv, const struct drm_display_mode *mode drm_hdmi_avi_infoframe_from_display_mode(&frame.avi, &priv->connector, mode); frame.avi.quantization_range = HDMI_QUANTIZATION_RANGE_FULL; + drm_hdmi_avi_infoframe_quant_range(&frame.avi, &priv->connector, mode, + priv->rgb_quant_range); tda998x_write_if(priv, DIP_IF_FLAGS_IF2, REG_IF2_HB0, &frame); } @@ -1426,6 +1429,16 @@ static void tda998x_bridge_mode_set(struct drm_bridge *bridge, u16 de_pix_s, de_pix_e; u8 reg, div, rep, sel_clk; + /* + * Since we are "computer" like, our source invariably produces + * full-range RGB. If the monitor supports full-range, then use + * it, otherwise reduce to limited-range. + */ + priv->rgb_quant_range = + priv->connector.display_info.rgb_quant_range_selectable ? + HDMI_QUANTIZATION_RANGE_FULL : + drm_default_rgb_quant_range(adjusted_mode); + /* * Internally TDA998x is using ITU-R BT.656 style sync but * we get VESA style sync. TDA998x is using a reference pixel @@ -1541,10 +1554,25 @@ static void tda998x_bridge_mode_set(struct drm_bridge *bridge, reg_write(priv, REG_PLL_SERIAL_2, PLL_SERIAL_2_SRL_NOSC(div) | PLL_SERIAL_2_SRL_PR(rep)); - /* set color matrix bypass flag: */ - reg_write(priv, REG_MAT_CONTRL, MAT_CONTRL_MAT_BP | - MAT_CONTRL_MAT_SC(1)); - reg_set(priv, REG_FEAT_POWERDOWN, FEAT_POWERDOWN_CSC); + /* set color matrix according to output rgb quant range */ + if (priv->rgb_quant_range == HDMI_QUANTIZATION_RANGE_LIMITED) { + static u8 tda998x_full_to_limited_range[] = { + MAT_CONTRL_MAT_SC(2), + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x03, 0x6f, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x03, 0x6f, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x03, 0x6f, + 0x00, 0x40, 0x00, 0x40, 0x00, 0x40 + }; + reg_clear(priv, REG_FEAT_POWERDOWN, FEAT_POWERDOWN_CSC); + reg_write_range(priv, REG_MAT_CONTRL, + tda998x_full_to_limited_range, + sizeof(tda998x_full_to_limited_range)); + } else { + reg_write(priv, REG_MAT_CONTRL, MAT_CONTRL_MAT_BP | + MAT_CONTRL_MAT_SC(1)); + reg_set(priv, REG_FEAT_POWERDOWN, FEAT_POWERDOWN_CSC); + } /* set BIAS tmds value: */ reg_write(priv, REG_ANA_GENERAL, 0x09); From 45a19dd397886a9591110c0e9ba7e058393a395d Mon Sep 17 00:00:00 2001 From: Russell King Date: Wed, 5 Dec 2018 15:51:45 +0000 Subject: [PATCH 13/13] drm/i2c: tda998x: add vendor specific infoframe support Add support for the vendor specific infoframe. Reviewed-by: Brian Starkey Tested-by: Sven Van Asbroeck Signed-off-by: Russell King --- drivers/gpu/drm/i2c/tda998x_drv.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c index 2d69aef556a5..3d368c43185f 100644 --- a/drivers/gpu/drm/i2c/tda998x_drv.c +++ b/drivers/gpu/drm/i2c/tda998x_drv.c @@ -877,6 +877,19 @@ tda998x_write_avi(struct tda998x_priv *priv, const struct drm_display_mode *mode tda998x_write_if(priv, DIP_IF_FLAGS_IF2, REG_IF2_HB0, &frame); } +static void tda998x_write_vsi(struct tda998x_priv *priv, + const struct drm_display_mode *mode) +{ + union hdmi_infoframe frame; + + if (drm_hdmi_vendor_infoframe_from_display_mode(&frame.vendor.hdmi, + &priv->connector, + mode)) + reg_clear(priv, REG_DIP_IF_FLAGS, DIP_IF_FLAGS_IF1); + else + tda998x_write_if(priv, DIP_IF_FLAGS_IF1, REG_IF1_HB0, &frame); +} + /* Audio support */ static const struct tda998x_audio_route tda998x_audio_route[AUDIO_ROUTE_NUM] = { @@ -1654,6 +1667,7 @@ static void tda998x_bridge_mode_set(struct drm_bridge *bridge, reg_set(priv, REG_TX33, TX33_HDMI); tda998x_write_avi(priv, adjusted_mode); + tda998x_write_vsi(priv, adjusted_mode); if (priv->sink_has_audio) tda998x_configure_audio(priv);