Re: [PATCH 1/2] drm/i2c: tda998x: adjust CTS_N audio pre-divider calculation

[Sven Van Asbroeck <thesven73@gmail.com>]

Russell, thank you so much for your patience, help and explanation, I really
appreciate it !

On Fri, Feb 22, 2019 at 3:16 PM Russell King - ARM Linux admin <linux@armlinux.org.uk> wrote:
>
> 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.
>

Yes, that would keep the current users in business without them having to
change anything.

Of course, poor souls like myself would have to patch, say, simple-card so we
could provide a bclk ratio in the devicetree. Which would then propagate down
to the tda998x via hdmi-codec. Which is fine by me.

Combining your two previous suggestions, I get the following. Now sample_width
can be removed from tda998x_audio_params, as it's no longer used.

Would this be a good start?

diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
index a7c39f39793f..a306994ecc79 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -893,19 +893,25 @@ 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;
 		}
 		break;
 
@@ -982,7 +988,7 @@ 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,
+		.bclk_ratio = daifmt->bclk_ratio,
 		.sample_rate = params->sample_rate,
 		.cea = params->cea,
 	};
@@ -1004,6 +1010,23 @@ static int tda998x_audio_hw_params(struct device *dev, void *data,
 			if (priv->audio_port[i].format == AFMT_I2S)
 				audio.config = priv->audio_port[i].config;
 		audio.format = AFMT_I2S;
+		if (!audio.bclk_ratio) {
+			/* 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;
+			}
+		}
 		break;
 	case HDMI_SPDIF:
 		for (i = 0; i < ARRAY_SIZE(priv->audio_port); i++)
diff --git a/include/drm/i2c/tda998x.h b/include/drm/i2c/tda998x.h
index 3cb25ccbe5e6..039e1d9af2e0 100644
--- a/include/drm/i2c/tda998x.h
+++ b/include/drm/i2c/tda998x.h
@@ -14,7 +14,7 @@ enum {
 struct tda998x_audio_params {
 	u8 config;
 	u8 format;
-	unsigned sample_width;
+	u8 bclk_ratio;
 	unsigned sample_rate;
 	struct hdmi_audio_infoframe cea;
 	u8 status[5];
diff --git a/include/sound/hdmi-codec.h b/include/sound/hdmi-codec.h
index 9483c55f871b..0fca69880dc3 100644
--- a/include/sound/hdmi-codec.h
+++ b/include/sound/hdmi-codec.h
@@ -42,6 +42,7 @@ struct hdmi_codec_daifmt {
 	unsigned int frame_clk_inv:1;
 	unsigned int bit_clk_master:1;
 	unsigned int frame_clk_master:1;
+	unsigned int bclk_ratio;
 };
 
 /*
diff --git a/sound/soc/codecs/hdmi-codec.c b/sound/soc/codecs/hdmi-codec.c
index d00734d31e04..d82f26854c90 100644
--- a/sound/soc/codecs/hdmi-codec.c
+++ b/sound/soc/codecs/hdmi-codec.c
@@ -524,6 +524,17 @@ static int hdmi_codec_hw_params(struct snd_pcm_substream *substream,
 				       &hcp->daifmt[dai->id], &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);
+
+	/* FIXME: some validation here would be good? */
+	hcp->daifmt[dai->id].bclk_ratio = ratio;
+
+	return 0;
+}
+
 static int hdmi_codec_set_fmt(struct snd_soc_dai *dai,
 			      unsigned int fmt)
 {
@@ -593,7 +604,11 @@ static int hdmi_codec_set_fmt(struct snd_soc_dai *dai,
 		}
 	}
 
-	hcp->daifmt[dai->id] = cf;
+	hcp->daifmt[dai->id].fmt = cf.fmt;
+	hcp->daifmt[dai->id].bit_clk_inv = cf.bit_clk_inv;
+	hcp->daifmt[dai->id].frame_clk_inv = cf.frame_clk_inv;
+	hcp->daifmt[dai->id].bit_clk_master = cf.bit_clk_master;
+	hcp->daifmt[dai->id].frame_clk_master = cf.frame_clk_master;
 
 	return ret;
 }
@@ -615,6 +630,7 @@ static const struct snd_soc_dai_ops hdmi_dai_ops = {
 	.startup	= hdmi_codec_startup,
 	.shutdown	= hdmi_codec_shutdown,
 	.hw_params	= hdmi_codec_hw_params,
+	.set_bclk_ratio	= hdmi_codec_set_bclk_ratio,
 	.set_fmt	= hdmi_codec_set_fmt,
 	.digital_mute	= hdmi_codec_digital_mute,
 };
-- 
2.17.1