Subject: Re: [PATCH] ASoC: hdmi-codec: add support for bclk_ratio

From: Russell King <[email-masked]>
Date: Fri, 18 Oct 2024 11:07:51 +0100

From: Russell King <[email-masked]>

Some HDMI codecs need to know the relationship between the I2S bit clock
and the I2S word clock in order to correctly generate the CTS value for
audio clock recovery on the sink.

Add support for this, but there are currently no callers of
snd_soc_dai_set_bclk_ratio(), we provide a default implementation that
uses the sample width to derive the ratio from the 8-bit aligned
sample size.  This reflects the derivation that is in TDA998x, which
we are going to convert to use this new support.

Signed-off-by: Russell King <[email-masked]>
---
 include/sound/hdmi-codec.h    |  7 +++++++
 sound/soc/codecs/hdmi-codec.c | 14 ++++++++++++++
 2 files changed, 21 insertions(+)

diff --git a/include/sound/hdmi-codec.h b/include/sound/hdmi-codec.h
index 9b162ac1e08e..32c6fd09b74a 100644
--- a/include/sound/hdmi-codec.h
+++ b/include/sound/hdmi-codec.h
@@ -63,6 +63,13 @@ struct hdmi_codec_ops {
 	 */
 	int (*audio_startup)(struct device *dev, void *data);
 
+	/*
+	 * Set the bit clock ratio, as needed for some HDMI bridges.
+	 * Optional.
+	 */
+	int (*set_bclk_ratio)(struct device *dev, void *data,
+			      unsigned int bclk_ratio);
+
 	/*
 	 * Configures HDMI-encoder for audio stream.
 	 * Having either prepare or hw_params is mandatory.
diff --git a/sound/soc/codecs/hdmi-codec.c b/sound/soc/codecs/hdmi-codec.c
index 09eef6042aad..0620447b7c8e 100644
--- a/sound/soc/codecs/hdmi-codec.c
+++ b/sound/soc/codecs/hdmi-codec.c
@@ -620,6 +620,19 @@ static int hdmi_codec_prepare(struct snd_pcm_substream *substream,
 				     cf, &hp);
 }
 
+static int hdmi_codec_set_bclk_ratio(struct snd_soc_dai *dai,
+				     unsigned int ratio)
+{
+	struct hdmi_codec_priv *hcp = snd_soc_dai_get_drvdata(dai);
+	int ret = -EOPNOTSUPP;
+
+	if (hcp->hcd.ops->set_bclk_ratio)
+		ret = hcp->hcd.ops->set_bclk_ratio(dai->dev->parent,
+						   hcp->hcd.data, ratio);
+
+	return ret;
+}
+
 static int hdmi_codec_i2s_set_fmt(struct snd_soc_dai *dai,
 				  unsigned int fmt)
 {
@@ -912,6 +925,7 @@ static const struct snd_soc_dai_ops hdmi_codec_i2s_dai_ops = {
 	.shutdown			= hdmi_codec_shutdown,
 	.hw_params			= hdmi_codec_hw_params,
 	.prepare			= hdmi_codec_prepare,
+	.set_bclk_ratio			= hdmi_codec_set_bclk_ratio,
 	.set_fmt			= hdmi_codec_i2s_set_fmt,
 	.mute_stream			= hdmi_codec_mute,
 	.pcm_new			= hdmi_codec_pcm_new,

---

From: Russell King <[email-masked]>
Date: Fri, 18 Oct 2024 11:07:51 +0100

Signed-off-by: Russell King <[email-masked]>
---
 drivers/gpu/drm/i2c/tda998x_drv.c | 57 ++++++++++++++++++++++++++++-----------
 1 file changed, 42 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
index d8d7de18dd65..15ea7b5f1f17 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -63,6 +63,7 @@ struct tda998x_priv {
 	u8 vip_cntrl_0;
 	u8 vip_cntrl_1;
 	u8 vip_cntrl_2;
+	u8 i2s_cts_n;
 	unsigned long tmds_clock;
 	struct tda998x_audio_settings audio;
 
@@ -962,25 +963,24 @@ static u8 tda998x_get_adiv(struct tda998x_priv *priv, unsigned int fs)
  *
  * 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,
+static int tda998x_derive_cts_n(struct tda998x_priv *priv, u8 *cts_n,
 				unsigned int ratio)
 {
 	switch (ratio) {
 	case 16:
-		settings->cts_n = CTS_N_M(3) | CTS_N_K(0);
+		*cts_n = CTS_N_M(3) | CTS_N_K(0);
 		break;
 	case 32:
-		settings->cts_n = CTS_N_M(3) | CTS_N_K(1);
+		*cts_n = CTS_N_M(3) | CTS_N_K(1);
 		break;
 	case 48:
-		settings->cts_n = CTS_N_M(3) | CTS_N_K(2);
+		*cts_n = CTS_N_M(3) | CTS_N_K(2);
 		break;
 	case 64:
-		settings->cts_n = CTS_N_M(3) | CTS_N_K(3);
+		*cts_n = CTS_N_M(3) | CTS_N_K(3);
 		break;
 	case 128:
-		settings->cts_n = CTS_N_M(0) | CTS_N_K(0);
+		*cts_n = CTS_N_M(0) | CTS_N_K(0);
 		break;
 	default:
 		dev_err(&priv->hdmi->dev, "unsupported bclk ratio %ufs\n",
@@ -1060,12 +1060,28 @@ static void tda998x_configure_audio(struct tda998x_priv *priv)
 	tda998x_write_aif(priv, &settings->cea);
 }
 
+static int tda998x_audio_startup(struct device *dev, void *data)
+{
+	struct tda998x_priv *priv = dev_get_drvdata(dev);
+
+	priv->i2s_cts_n = 0xff;
+
+	return 0;
+}
+
+static int tda998x_audio_bclk_ratio(struct device *dev, void *data,
+				    unsigned int ratio)
+{
+	struct tda998x_priv *priv = dev_get_drvdata(dev);
+
+	return tda998x_derive_cts_n(priv, &priv->i2s_cts_n, ratio);
+}
+
 static int tda998x_audio_hw_params(struct device *dev, void *data,
 				   struct hdmi_codec_daifmt *daifmt,
 				   struct hdmi_codec_params *params)
 {
 	struct tda998x_priv *priv = dev_get_drvdata(dev);
-	unsigned int bclk_ratio;
 	bool spdif = daifmt->fmt == HDMI_SPDIF;
 	int ret;
 	struct tda998x_audio_settings audio = {
@@ -1108,10 +1124,19 @@ static int tda998x_audio_hw_params(struct device *dev, void *data,
 	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)
-		return ret;
+	if (spdif) {
+		tda998x_derive_cts_n(priv, &audio.cts_n, 64);
+	} else if (priv->i2s_cts_n != 0xff) {
+		audio.cts_n = priv->i2s_cts_n;
+	} else {
+		dev_warn_once(&priv->hdmi->dev,
+			      "I2S bit clock ratio not set, defaulting to 2x sample_width.\n"
+			      "Please call snd_soc_dai_set_bclk_ratio() for this codec.\n");
+		ret = tda998x_derive_cts_n(priv, &audio.cts_n,
+					   params->sample_width * 2);
+		if (ret < 0)
+			return ret;
+	}
 
 	mutex_lock(&priv->audio_mutex);
 	priv->audio = audio;
@@ -1161,6 +1186,8 @@ static int tda998x_audio_get_eld(struct device *dev, void *data,
 }
 
 static const struct hdmi_codec_ops audio_codec_ops = {
+	.audio_startup = tda998x_audio_startup,
+	.set_bclk_ratio = tda998x_audio_bclk_ratio,
 	.hw_params = tda998x_audio_hw_params,
 	.audio_shutdown = tda998x_audio_shutdown,
 	.mute_stream = tda998x_audio_mute_stream,
@@ -1762,7 +1789,7 @@ 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, route;
+		unsigned int route;
 		bool spdif = p->audio_params.format == AFMT_SPDIF;
 
 		route = AUDIO_ROUTE_I2S + spdif;
@@ -1776,8 +1803,8 @@ static int tda998x_set_config(struct tda998x_priv *priv,
 		priv->audio.ena_ap = p->audio_params.config;
 		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 tda998x_derive_cts_n(priv, &priv->audio.cts_n,
+					    p->audio_params.sample_width * 2);
 	}
 
 	return 0;

---

From: Russell King <[email-masked]>
Date: Fri, 18 Oct 2024 11:07:51 +0100

Add bridge timing information so that bridge users can figure out the
timing parameters that are necessary for TDA998x.

Signed-off-by: Russell King <[email-masked]>
---
 drivers/gpu/drm/i2c/tda998x_drv.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
index 15ea7b5f1f17..3f6dfc08469a 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -1715,6 +1715,18 @@ static const struct drm_bridge_funcs tda998x_bridge_funcs = {
 	.enable = tda998x_bridge_enable,
 };
 
+static const struct drm_bridge_timings tda9989_timings = {
+	.input_bus_flags = DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE,
+	.setup_time_ps = 1500,
+	.hold_time_ps = 1000,
+};
+
+static const struct drm_bridge_timings tda19988_timings = {
+	.input_bus_flags = DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE,
+	.setup_time_ps = 1600,
+	.hold_time_ps = 1200,
+};
+
 /* I2C driver functions */
 
 static int tda998x_get_audio_ports(struct tda998x_priv *priv,
@@ -2019,6 +2031,17 @@ static int tda998x_create(struct device *dev)
 	priv->bridge.of_node = dev->of_node;
 #endif
 
+	switch (priv->rev) {
+	case TDA9989N2:
+	case TDA19989:
+	case TDA19989N2:
+		priv->bridge.timings = &tda9989_timings;
+		break;
+	case TDA19988:
+		priv->bridge.timings = &tda19988_timings;
+		break;
+	}
+
 	drm_bridge_add(&priv->bridge);
 
 	return 0;

---

From: Russell King <[email-masked]>
Date: Fri, 18 Oct 2024 11:07:51 +0100

Add support for writing the SPD infoframe to the TDA998x.  Identify us
as "Generic" vendor "PC" product, and as "PC general" source device
information.

Signed-off-by: Russell King <[email-masked]>
---
 drivers/gpu/drm/i2c/tda998x_drv.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
index 3f6dfc08469a..c06356e7d7a1 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -856,6 +856,23 @@ static void tda998x_write_aif(struct tda998x_priv *priv,
 	tda998x_write_if(priv, DIP_IF_FLAGS_IF4, REG_IF4_HB0, &frame);
 }
 
+static void tda998x_write_spd(struct tda998x_priv *priv)
+{
+	union hdmi_infoframe frame;
+	int ret;
+
+	ret = hdmi_spd_infoframe_init(&frame.spd, "Generic", "PC");
+	if (ret < 0) {
+		dev_err(&priv->hdmi->dev, "failed to fill SPD infoframe: %d\n",
+			ret);
+		return;
+	}
+
+	frame.spd.sdi = HDMI_SPD_SDI_PC;
+
+	tda998x_write_if(priv, DIP_IF_FLAGS_IF3, REG_IF3_HB0, &frame);
+}
+
 static void
 tda998x_write_avi(struct tda998x_priv *priv, const struct drm_display_mode *mode)
 {
@@ -1697,6 +1714,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_spd(priv);
 		tda998x_write_vsi(priv, adjusted_mode);
 
 		if (priv->sink_has_audio)