[PATCH 00/13] tda998x updates

[PATCH 00/13] tda998x updates

[Russell King - ARM Linux admin]

This series represents development work collected over the last six
months to improve the TDA998x driver, particularly for the audio
side.  These patches can be found in my "drm-tda998x-devel" branch
at git://git.armlinux.org.uk/~rmk/linux-arm.git

- Introduce an audio_settings structure so we can store the derived
  register settings independently of the audio parameters.
- Add support for the different I2S flavours.
- Improve the calculation of the audio clock divisor, the old code
  seemed to prevent combinations of video mode and audio sample rate
  from working.  Document what we don't know, what assumptions we are
  making, what has been found through experimentation, and what we are
  actually doing.
- Add calculation of the CTS n and k values depending on the bit clock
  to sample rate ratio (bclk_ratio) - we assume a bclk_ratio
  appropriate for the bus format that gives us the values we are using
  prior to this change to maintain compatibility.
- Move the "ena_ap" register value into the audio_settings structure.
- Eliminate the audio port array and repetitive searching of the array,
  instead looking up the "ena_ap" value by format - the DT binding only
  supports one setting per format, so old approach was not a good
  design.
- There is no need to set the two fields of the AIP_CLKSEL register
  independently of each other, so just write the register once while
  setting up audio.
- Deal with the format specific audio routing configuration when the
  audio settings are configured, rather than when programming the
  TDA998x, which means we can do all the validation at configuration
  time, rather than spreading it into other paths like modeset, where
  failures can't be reported.
- Since tda998x_configure_audio() is called from paths where failure
  is not an option, and we have eliminated the configuration dependent
  failures, we can make this functions return type void.  This allows
  simplification of tda998x_audio_hw_params() within the mutex
  protected region.
- We no longer need to store the full audio params within the audio
  settings structure, so eliminate it, only storing in the audio
  settings structure what we need to actually program the hardware.

- Add support for pixel repeated modes, tested on a Panasonic TV that
  supports these formats.
- Add bridge timing specifications for TDA9989 and TDA19988 devices,
  although we have no current users of this data it may be useful to
  complete the information passed from the bridge driver, especially
  where the pixel clock edge needs configuration.
- Add support for selecting the appropriate RGB quantisation range
  depending on the sink capabilities, and avoid sending full range RGB
  when the sink only supports limited range RGB for the mode.  We do
  this by enabling the TDA998x's colour scaling matrix and applying
  the appropriate constants.

 drivers/gpu/drm/i2c/tda998x_drv.c | 459 ++++++++++++++++++++++++++------------
 1 file changed, 311 insertions(+), 148 deletions(-)

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 01/13] drm/i2c: tda998x: introduce tda998x_audio_settings

[Russell King]

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.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 drivers/gpu/drm/i2c/tda998x_drv.c | 56 ++++++++++++++++++++++-----------------
 1 file changed, 31 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
index 7f34601bb515..e478633d9a7a 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;
@@ -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, &params->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:
@@ -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)
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 01/13] drm/i2c: tda998x: introduce tda998x_audio_settings

[Sven Van Asbroeck]

On Tue, Jun 11, 2019 at 7:01 AM Russell King <rmk+kernel@armlinux.org.uk> wrote:
>
> 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.
>
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> ---

Nit:
Fix compile errors? Note that these errors disappear if patch 2/13
is applied, but maybe we want to make sure that every patch compiles
so git bisect does not break?

drivers/gpu/drm/i2c/tda998x_drv.c: In function ‘tda998x_audio_hw_params’:
drivers/gpu/drm/i2c/tda998x_drv.c:1011:10: error: ‘struct
tda998x_audio_settings’ has no member named ‘config’
     audio.config = priv->audio_port[i].config;
          ^
drivers/gpu/drm/i2c/tda998x_drv.c:1012:8: error: ‘struct
tda998x_audio_settings’ has no member named ‘format’
   audio.format = AFMT_I2S;
        ^
drivers/gpu/drm/i2c/tda998x_drv.c:1017:10: error: ‘struct
tda998x_audio_settings’ has no member named ‘config’
     audio.config = priv->audio_port[i].config;
          ^
drivers/gpu/drm/i2c/tda998x_drv.c:1018:8: error: ‘struct
tda998x_audio_settings’ has no member named ‘format’
   audio.format = AFMT_SPDIF;
        ^
drivers/gpu/drm/i2c/tda998x_drv.c:1025:11: error: ‘struct
tda998x_audio_settings’ has no member named ‘config’
  if (audio.config == 0) {
           ^
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 01/13] drm/i2c: tda998x: introduce tda998x_audio_settings

[Russell King - ARM Linux admin]

On Wed, Jun 12, 2019 at 11:24:46AM -0400, Sven Van Asbroeck wrote:
> On Tue, Jun 11, 2019 at 7:01 AM Russell King <rmk+kernel@armlinux.org.uk> wrote:
> >
> > 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.
> >
> > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> > ---
> 
> Nit:
> Fix compile errors? Note that these errors disappear if patch 2/13
> is applied, but maybe we want to make sure that every patch compiles
> so git bisect does not break?
> 
> drivers/gpu/drm/i2c/tda998x_drv.c: In function ‘tda998x_audio_hw_params’:
> drivers/gpu/drm/i2c/tda998x_drv.c:1011:10: error: ‘struct
> tda998x_audio_settings’ has no member named ‘config’
>      audio.config = priv->audio_port[i].config;
>           ^
> drivers/gpu/drm/i2c/tda998x_drv.c:1012:8: error: ‘struct
> tda998x_audio_settings’ has no member named ‘format’
>    audio.format = AFMT_I2S;
>         ^
> drivers/gpu/drm/i2c/tda998x_drv.c:1017:10: error: ‘struct
> tda998x_audio_settings’ has no member named ‘config’
>      audio.config = priv->audio_port[i].config;
>           ^
> drivers/gpu/drm/i2c/tda998x_drv.c:1018:8: error: ‘struct
> tda998x_audio_settings’ has no member named ‘format’
>    audio.format = AFMT_SPDIF;
>         ^
> drivers/gpu/drm/i2c/tda998x_drv.c:1025:11: error: ‘struct
> tda998x_audio_settings’ has no member named ‘config’
>   if (audio.config == 0) {
>            ^

Thanks for reporting.  All fixed.

However, I'm concerned that the 0-day builder never found these despite
this code being published for many weeks.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 02/13] drm/i2c: tda998x: implement different I2S flavours

[Russell King]

Add support for the left and right justified I2S formats as well as the
more tranditional "Philips" I2S format.

Tested-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 drivers/gpu/drm/i2c/tda998x_drv.c | 53 +++++++++++++++++++++++++--------------
 1 file changed, 34 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
index e478633d9a7a..e211cbe44580 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.config = priv->audio_port[i].config;
-		audio.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.config = priv->audio_port[i].config;
-		audio.format = AFMT_SPDIF;
+		audio.i2s_format = 0;
 		break;
 	default:
 		dev_err(dev, "%s: Invalid format %d\n", __func__, daifmt->fmt);
 		return -EINVAL;
 	}
 
-	if (audio.config == 0) {
+	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)
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 03/13] drm/i2c: tda998x: improve programming of audio divisor

[Russell King]

Improve the selection of the audio clock divisor so that more modes
and sample rates work.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 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 e211cbe44580..78a2b815a8de 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);
 
 	/*
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 03/13] drm/i2c: tda998x: improve programming of audio divisor

[Sven Van Asbroeck]

On Tue, Jun 11, 2019 at 7:02 AM Russell King <rmk+kernel@armlinux.org.uk> wrote:
>
> Improve the selection of the audio clock divisor so that more modes
> and sample rates work.
>
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> ---

+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;

Doesn't this function need an error return in case min_audio_clk > ser_clk ?
Or is that a situation that can never arise?
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 03/13] drm/i2c: tda998x: improve programming of audio divisor

[Russell King - ARM Linux admin]

On Wed, Jun 12, 2019 at 11:25:59AM -0400, Sven Van Asbroeck wrote:
> On Tue, Jun 11, 2019 at 7:02 AM Russell King <rmk+kernel@armlinux.org.uk> wrote:
> >
> > Improve the selection of the audio clock divisor so that more modes
> > and sample rates work.
> >
> > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> > ---
> 
> +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;
> 
> Doesn't this function need an error return in case min_audio_clk > ser_clk ?
> Or is that a situation that can never arise?

It's possible it could arise.  For example, if we have a 192kHz sample
rate, and a tmds clock slower than 24.576MHz.

In such a case, we will select AUDIO_DIV_SERCLK_1 as the divisor, which
is a legal value.  The result _may_ be audio not working (which is what
already happens today when we get this setting wrong.)

If we were to return an error, there's no way to handle that error, so
what's the point of returning an error?

The results of this function match what the Philips firmware uses for
this register for all standard TMDS clocks and sample rates, so it's
not a problem that I'm particularly concerned about at this point.
The only way around this would be to increase the TMDS clock, which
means using pixel doubling tricks, and therefore a mode set.  I don't
think any HDMI driver has the capability to deal with that yet.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 04/13] drm/i2c: tda998x: derive CTS_N value from aclk sample rate ratio

[Russell King]

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.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 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 78a2b815a8de..bf75414a8c63 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;
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 04/13] drm/i2c: tda998x: derive CTS_N value from aclk sample rate ratio

[Sven Van Asbroeck]

On Tue, Jun 11, 2019 at 7:02 AM Russell King <rmk+kernel@armlinux.org.uk> wrote:
>
> 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.
>
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> ---

@@ -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);

Won't this make the platform_data path fail all the time?
Also, the platform_data path doesn't appear to instantiate the HDMI_CODEC,
so tda audio support would be completely missing in this case?
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 04/13] drm/i2c: tda998x: derive CTS_N value from aclk sample rate ratio

[Russell King - ARM Linux admin]

On Wed, Jun 12, 2019 at 11:27:16AM -0400, Sven Van Asbroeck wrote:
> On Tue, Jun 11, 2019 at 7:02 AM Russell King <rmk+kernel@armlinux.org.uk> wrote:
> >
> > 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.
> >
> > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> > ---
> 
> @@ -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);
> 
> Won't this make the platform_data path fail all the time?
> Also, the platform_data path doesn't appear to instantiate the HDMI_CODEC,
> so tda audio support would be completely missing in this case?

The platform data path has never supported the HDMI codec way of doing
things, so that really isn't a concern here.  The platform data way
was sufficient to allow SPDIF streams to work with a static setup of
the TDA998x, and has never been intended to support anything beyond
that.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 04/13] drm/i2c: tda998x: derive CTS_N value from aclk sample rate ratio

[Sven Van Asbroeck]

On Wed, Jun 12, 2019 at 12:28 PM Russell King - ARM Linux admin
<linux@armlinux.org.uk> wrote:
>
> The platform data path has never supported the HDMI codec way of doing
> things, so that really isn't a concern here.  The platform data way
> was sufficient to allow SPDIF streams to work with a static setup of
> the TDA998x, and has never been intended to support anything beyond
> that.

Thank you, I am not using the platform data path, so I had no idea.

Wouldn't the current code always fail on the platform data path though?

create() calls tda998x_set_config() in the platform data case.
If the audio_params.format is used (i.e. the platform data configures
audio), the function then returns the return value of tda998x_derive_cts_n(),
which is a positive divider (can be 0 if /1).

Then in create(): if (ret) goto fail;
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 04/13] drm/i2c: tda998x: derive CTS_N value from aclk sample rate ratio

[Russell King - ARM Linux admin]

On Wed, Jun 12, 2019 at 12:37:57PM -0400, Sven Van Asbroeck wrote:
> On Wed, Jun 12, 2019 at 12:28 PM Russell King - ARM Linux admin
> <linux@armlinux.org.uk> wrote:
> >
> > The platform data path has never supported the HDMI codec way of doing
> > things, so that really isn't a concern here.  The platform data way
> > was sufficient to allow SPDIF streams to work with a static setup of
> > the TDA998x, and has never been intended to support anything beyond
> > that.
> 
> Thank you, I am not using the platform data path, so I had no idea.
> 
> Wouldn't the current code always fail on the platform data path though?
> 
> create() calls tda998x_set_config() in the platform data case.
> If the audio_params.format is used (i.e. the platform data configures
> audio), the function then returns the return value of tda998x_derive_cts_n(),
> which is a positive divider (can be 0 if /1).

I think you're confusing tda998x_derive_cts_n() and tda998x_get_adiv().
tda998x_derive_cts_n() only returns 0 or -EINVAL.

> 
> Then in create(): if (ret) goto fail;
> 

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 04/13] drm/i2c: tda998x: derive CTS_N value from aclk sample rate ratio

[Sven Van Asbroeck]

On Wed, Jun 12, 2019 at 12:42 PM Russell King - ARM Linux admin
<linux@armlinux.org.uk> wrote:
>
> I think you're confusing tda998x_derive_cts_n() and tda998x_get_adiv().
> tda998x_derive_cts_n() only returns 0 or -EINVAL.
>

True. Apologies for the confusion.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 05/13] drm/i2c: tda998x: store audio port enable in settings

[Russell King]

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.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 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 bf75414a8c63..21359bc66d60 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;
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 06/13] drm/i2c: tda998x: index audio port enable config by route type

[Russell King]

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.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 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 21359bc66d60..7ab12911f482 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);
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 07/13] drm/i2c: tda998x: configure both fields of AIP_CLKSEL together

[Russell King]

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.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 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 7ab12911f482..8b79619ff152 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);
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 08/13] drm/i2c: tda998x: move audio routing configuration

[Russell King]

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.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 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 8b79619ff152..14d1672301ae 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;
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 08/13] drm/i2c: tda998x: move audio routing configuration

[Sven Van Asbroeck]

On Tue, Jun 11, 2019 at 7:02 AM Russell King <rmk+kernel@armlinux.org.uk> wrote:
>
> 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.
>
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> ---

+/* 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;
+}

Nit: priv is nearly unused in this function.
Maybe delegate the error log to the caller, in that case we could just pass
route and const audio_port_enable to the function. Instead of passing in the
'kitchen sink' priv ?
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 08/13] drm/i2c: tda998x: move audio routing configuration

[Russell King - ARM Linux admin]

On Wed, Jun 12, 2019 at 11:36:59AM -0400, Sven Van Asbroeck wrote:
> On Tue, Jun 11, 2019 at 7:02 AM Russell King <rmk+kernel@armlinux.org.uk> wrote:
> >
> > 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.
> >
> > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> > ---
> 
> +/* 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;
> +}
> 
> Nit: priv is nearly unused in this function.
> Maybe delegate the error log to the caller, in that case we could just pass
> route and const audio_port_enable to the function. Instead of passing in the
> 'kitchen sink' priv ?

I don't think that's worth doing.  This way, compilers are free to
emit code to bounds-check the audio_port_enable access since they
know that it is a defined size.  Passing in a const pointer ca
mean that check has to be avoided.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 09/13] drm/i2c: tda998x: clean up tda998x_configure_audio()

[Russell King]

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.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 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 14d1672301ae..4b5491e20ab6 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,
-			     struct hdmi_audio_infoframe *cea)
+static void tda998x_write_aif(struct tda998x_priv *priv,
+			      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)
 {
+	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);
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 09/13] drm/i2c: tda998x: clean up tda998x_configure_audio()

[Sven Van Asbroeck]

On Tue, Jun 11, 2019 at 7:02 AM Russell King <rmk+kernel@armlinux.org.uk> wrote:
>
> 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.
>
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> ---

Nit:

+static void tda998x_configure_audio(struct tda998x_priv *priv)
 {
+       struct tda998x_audio_settings *settings = &priv->audio;

settings could be const ?

-static int tda998x_write_aif(struct tda998x_priv *priv,
-                            struct hdmi_audio_infoframe *cea)
+static void tda998x_write_aif(struct tda998x_priv *priv,
+                             struct hdmi_audio_infoframe *cea)

cea could be const ?
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 10/13] drm/i2c: tda998x: get rid of params in audio settings

[Russell King]

Get rid of the tda998x_audio_params structure in audio_settings, which
is now just used for platform data.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 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 4b5491e20ab6..783cbc9c97b2 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;
 
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 11/13] drm/i2c: tda998x: add support for pixel repeated modes

[Russell King]

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.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 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 783cbc9c97b2..417949fc69d1 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,
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 12/13] drm/i2c: tda998x: add bridge timing information

[Russell King]

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

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 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 417949fc69d1..372c462323cf 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -1643,6 +1643,18 @@ static const struct drm_bridge_funcs tda998x_bridge_funcs = {
 	.enable = tda998x_bridge_enable,
 };
 
+static const struct drm_bridge_timings tda9989_timings = {
+	.sampling_edge = DRM_BUS_FLAG_PIXDATA_POSEDGE,
+	.setup_time_ps = 1500,
+	.hold_time_ps = 1000,
+};
+
+static const struct drm_bridge_timings tda19988_timings = {
+	.sampling_edge = DRM_BUS_FLAG_PIXDATA_POSEDGE,
+	.setup_time_ps = 1600,
+	.hold_time_ps = 1200,
+};
+
 /* I2C driver functions */
 
 static int tda998x_get_audio_ports(struct tda998x_priv *priv,
@@ -1948,6 +1960,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;
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 12/13] drm/i2c: tda998x: add bridge timing information

[Sven Van Asbroeck]

On Tue, Jun 11, 2019 at 7:04 AM Russell King <rmk+kernel@armlinux.org.uk> wrote:
>
> Add bridge timing information so that bridge users can figure out the
> timing parameters that are necessary for TDA998x.
>
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> ---

+static const struct drm_bridge_timings tda9989_timings = {
+       .sampling_edge = DRM_BUS_FLAG_PIXDATA_POSEDGE,
+       .setup_time_ps = 1500,
+       .hold_time_ps = 1000,
+};
+
+static const struct drm_bridge_timings tda19988_timings = {
+       .sampling_edge = DRM_BUS_FLAG_PIXDATA_POSEDGE,
+       .setup_time_ps = 1600,
+       .hold_time_ps = 1200,
+};

Need to port these to 5.1 kernel: sampling_edge -> input_bus_flags ?
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 12/13] drm/i2c: tda998x: add bridge timing information

[Russell King - ARM Linux admin]

On Wed, Jun 12, 2019 at 11:38:06AM -0400, Sven Van Asbroeck wrote:
> On Tue, Jun 11, 2019 at 7:04 AM Russell King <rmk+kernel@armlinux.org.uk> wrote:
> >
> > Add bridge timing information so that bridge users can figure out the
> > timing parameters that are necessary for TDA998x.
> >
> > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> > ---
> 
> +static const struct drm_bridge_timings tda9989_timings = {
> +       .sampling_edge = DRM_BUS_FLAG_PIXDATA_POSEDGE,
> +       .setup_time_ps = 1500,
> +       .hold_time_ps = 1000,
> +};
> +
> +static const struct drm_bridge_timings tda19988_timings = {
> +       .sampling_edge = DRM_BUS_FLAG_PIXDATA_POSEDGE,
> +       .setup_time_ps = 1600,
> +       .hold_time_ps = 1200,
> +};
> 
> Need to port these to 5.1 kernel: sampling_edge -> input_bus_flags ?

5.1 still has it as "sampling_edge", but 5.2-rc changes this, so
I'll drop this patch from this series for now.  Thanks for pointing
that out.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 13/13] drm/i2c: tda998x: improve correctness of quantisation range

[Russell King]

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 <brian.starkey@arm.com>
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 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 372c462323cf..9366be6f6aa9 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);
 }
@@ -1427,6 +1430,16 @@ static void tda998x_bridge_mode_set(struct drm_bridge *bridge,
 	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
 	 * relative to ITU to sync to the input frame and for output
@@ -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);
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 00/13] tda998x updates

[Sven Van Asbroeck]

On Tue, Jun 11, 2019 at 7:01 AM Russell King - ARM Linux admin
<linux@armlinux.org.uk> wrote:
>
> This series represents development work collected over the last six
> months to improve the TDA998x driver, particularly for the audio
> side.  These patches can be found in my "drm-tda998x-devel" branch
> at git://git.armlinux.org.uk/~rmk/linux-arm.git

Thank you Russell !!

I tested this patch set on my platform: imx6q ssi codec driving a tda19988.
No issues as far as I can tell.

Note that I still have to 'hack' the bclk_ratio in the tda driver to correspond
with the wire format that the imx6q ssi is generating. But that's a known
issue.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 00/13] tda998x updates

[Russell King - ARM Linux admin]

On Wed, Jun 12, 2019 at 11:40:43AM -0400, Sven Van Asbroeck wrote:
> On Tue, Jun 11, 2019 at 7:01 AM Russell King - ARM Linux admin
> <linux@armlinux.org.uk> wrote:
> >
> > This series represents development work collected over the last six
> > months to improve the TDA998x driver, particularly for the audio
> > side.  These patches can be found in my "drm-tda998x-devel" branch
> > at git://git.armlinux.org.uk/~rmk/linux-arm.git
> 
> Thank you Russell !!
> 
> I tested this patch set on my platform: imx6q ssi codec driving a tda19988.
> No issues as far as I can tell.
> 
> Note that I still have to 'hack' the bclk_ratio in the tda driver to correspond
> with the wire format that the imx6q ssi is generating. But that's a known
> issue.

Yep, I avoided posting those patches so that this set can get merged,
which moves us towards supporting more I2S formats.  Once we have
agreement with these (which should be easy) we can then resume trying
to resolve the hdmi-codec side of the problem.

I'll repost the patch set later today, I'm intending to drop the
bridge timing patch from the set for now, but add the patch for the
vendor-specific infoframe that can go upstream in its place.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 00/13] tda998x updates

[Russell King - ARM Linux admin]

This series represents development work collected over the last six
months to improve the TDA998x driver, particularly for the audio
side.  These patches can be found in my "drm-tda998x-devel" branch
at git://git.armlinux.org.uk/~rmk/linux-arm.git

- Introduce an audio_settings structure so we can store the derived
  register settings independently of the audio parameters.
- Add support for the different I2S flavours.
- Improve the calculation of the audio clock divisor, the old code
  seemed to prevent combinations of video mode and audio sample rate
  from working.  Document what we don't know, what assumptions we are
  making, what has been found through experimentation, and what we are
  actually doing.
- Add calculation of the CTS n and k values depending on the bit clock
  to sample rate ratio (bclk_ratio) - we assume a bclk_ratio
  appropriate for the bus format that gives us the values we are using
  prior to this change to maintain compatibility.
- Move the "ena_ap" register value into the audio_settings structure.
- Eliminate the audio port array and repetitive searching of the array,
  instead looking up the "ena_ap" value by format - the DT binding only
  supports one setting per format, so old approach was not a good
  design.
- There is no need to set the two fields of the AIP_CLKSEL register
  independently of each other, so just write the register once while
  setting up audio.
- Deal with the format specific audio routing configuration when the
  audio settings are configured, rather than when programming the
  TDA998x, which means we can do all the validation at configuration
  time, rather than spreading it into other paths like modeset, where
  failures can't be reported.
- Since tda998x_configure_audio() is called from paths where failure
  is not an option, and we have eliminated the configuration dependent
  failures, we can make this functions return type void.  This allows
  simplification of tda998x_audio_hw_params() within the mutex
  protected region.
- We no longer need to store the full audio params within the audio
  settings structure, so eliminate it, only storing in the audio
  settings structure what we need to actually program the hardware.

- Add support for pixel repeated modes, tested on a Panasonic TV that
  supports these formats.
- Add support for selecting the appropriate RGB quantisation range
  depending on the sink capabilities, and avoid sending full range RGB
  when the sink only supports limited range RGB for the mode.  We do
  this by enabling the TDA998x's colour scaling matrix and applying
  the appropriate constants.
- Add support for setting the vendor specific infoframe, which ensures
  that we supply the information the sink expects for 3D modes (we
  support a htotal/vtotal up to 8191x2047, which allows for at least
  side-by-side 3D.)

 drivers/gpu/drm/i2c/tda998x_drv.c | 450 +++++++++++++++++++++++++-------------
 1 file changed, 302 insertions(+), 148 deletions(-)

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 01/13] drm/i2c: tda998x: introduce tda998x_audio_settings

[Russell King]

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.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 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, &params->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)
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 01/13] drm/i2c: tda998x: introduce tda998x_audio_settings

[Sven Van Asbroeck]

Nit: checkpatch.pl appears unhappy with this patch:

WARNING: line over 80 characters
#222: FILE: drivers/gpu/drm/i2c/tda998x_drv.c:1011:
+ audio.params.config = priv->audio_port[i].config;

WARNING: line over 80 characters
#230: FILE: drivers/gpu/drm/i2c/tda998x_drv.c:1017:
+ audio.params.config = priv->audio_port[i].config;

total: 0 errors, 2 warnings, 178 lines checked
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 01/13] drm/i2c: tda998x: introduce tda998x_audio_settings

[Russell King - ARM Linux admin]

On Thu, Jun 13, 2019 at 02:48:47PM -0400, Sven Van Asbroeck wrote:
> Nit: checkpatch.pl appears unhappy with this patch:
> 
> WARNING: line over 80 characters
> #222: FILE: drivers/gpu/drm/i2c/tda998x_drv.c:1011:
> + audio.params.config = priv->audio_port[i].config;
> 
> WARNING: line over 80 characters
> #230: FILE: drivers/gpu/drm/i2c/tda998x_drv.c:1017:
> + audio.params.config = priv->audio_port[i].config;
> 
> total: 0 errors, 2 warnings, 178 lines checked

Considering that code is going away in the very next patch, those
warnings are not something I'm concerned about - the extra hassle
of formatting the code so checkpatch is happy only to remove it
in the next patch is not worth the effort.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 02/13] drm/i2c: tda998x: implement different I2S flavours

[Russell King]

Add support for the left and right justified I2S formats as well as the
more tranditional "Philips" I2S format.

Tested-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 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)
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 03/13] drm/i2c: tda998x: improve programming of audio divisor

[Russell King]

Improve the selection of the audio clock divisor so that more modes
and sample rates work.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 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);
 
 	/*
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 04/13] drm/i2c: tda998x: derive CTS_N value from aclk sample rate ratio

[Russell King]

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.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 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;
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 05/13] drm/i2c: tda998x: store audio port enable in settings

[Russell King]

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.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 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;
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 06/13] drm/i2c: tda998x: index audio port enable config by route type

[Russell King]

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.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 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);
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 07/13] drm/i2c: tda998x: configure both fields of AIP_CLKSEL together

[Russell King]

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.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 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);
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 08/13] drm/i2c: tda998x: move audio routing configuration

[Russell King]

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.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 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;
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 09/13] drm/i2c: tda998x: clean up tda998x_configure_audio()

[Russell King]

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.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 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);
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 10/13] drm/i2c: tda998x: get rid of params in audio settings

[Russell King]

Get rid of the tda998x_audio_params structure in audio_settings, which
is now just used for platform data.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 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;
 
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 11/13] drm/i2c: tda998x: add support for pixel repeated modes

[Russell King]

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.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 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,
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 12/13] drm/i2c: tda998x: improve correctness of quantisation range

[Russell King]

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 <brian.starkey@arm.com>
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 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);
 }
@@ -1427,6 +1430,16 @@ static void tda998x_bridge_mode_set(struct drm_bridge *bridge,
 	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
 	 * relative to ITU to sync to the input frame and for output
@@ -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);
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 13/13] drm/i2c: tda998x: add vendor specific infoframe support

[Russell King]

Add support for the vendor specific infoframe.

Reviewed-by: Brian Starkey <brian.starkey@arm.com>
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 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);
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 00/13] tda998x updates

[Sven Van Asbroeck]

On Thu, Jun 13, 2019 at 10:29 AM Russell King - ARM Linux admin
<linux@armlinux.org.uk> wrote:
>
> This series represents development work collected over the last six
> months to improve the TDA998x driver, particularly for the audio
> side.  These patches can be found in my "drm-tda998x-devel" branch
> at git://git.armlinux.org.uk/~rmk/linux-arm.git

For the whole patchset, after 'hacking' the bclk_ratio to correspond with
my imx6q ssi's wire format:

Tested-by: Sven Van Asbroeck <TheSven73@gmail.com>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 00/13] tda998x updates

[Russell King - ARM Linux admin]

On Thu, Jun 13, 2019 at 03:51:06PM -0400, Sven Van Asbroeck wrote:
> On Thu, Jun 13, 2019 at 10:29 AM Russell King - ARM Linux admin
> <linux@armlinux.org.uk> wrote:
> >
> > This series represents development work collected over the last six
> > months to improve the TDA998x driver, particularly for the audio
> > side.  These patches can be found in my "drm-tda998x-devel" branch
> > at git://git.armlinux.org.uk/~rmk/linux-arm.git
> 
> For the whole patchset, after 'hacking' the bclk_ratio to correspond with
> my imx6q ssi's wire format:
> 
> Tested-by: Sven Van Asbroeck <TheSven73@gmail.com>

Thanks.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel