Re: [RESEND] drm/i2c: tda998x: Reset the I2S_FORMAT (Page0, Reg 0xfc) to it's default

[Russell King - ARM Linux admin <linux@armlinux.org.uk>]

On Fri, Feb 22, 2019 at 05:20:15PM +0200, Peter Ujfalusi wrote:
> > and I think we should implement at least setting the I2S
> > register format from the hdmi_codec_daifmt data.
> 
> Yes, that needs to be done for sure, but without data sheet with
> register descriptions I would not attempt to do that.

How about this, although it's not particularly pretty.

diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
index a7c39f39793f..c53128bb40fd 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -242,7 +242,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)
@@ -872,14 +874,14 @@ static int
 tda998x_configure_audio(struct tda998x_priv *priv,
 			struct tda998x_audio_params *params)
 {
-	u8 buf[6], clksel_aip, clksel_fs, cts_n, adiv;
+	u8 buf[6], clksel_aip, clksel_fs, cts_n, adiv, i2s_fmt;
 	u32 n;
 
 	/* Enable audio ports */
 	reg_write(priv, REG_ENA_AP, params->config);
 
 	/* Set audio input source */
-	switch (params->format) {
+	switch (params->format & AFMT_MASK) {
 	case AFMT_SPDIF:
 		reg_write(priv, REG_ENA_ACLK, 0);
 		reg_write(priv, REG_MUX_AP, MUX_AP_SELECT_SPDIF);
@@ -907,6 +909,19 @@ tda998x_configure_audio(struct tda998x_priv *priv,
 			cts_n = CTS_N_M(3) | CTS_N_K(3);
 			break;
 		}
+
+		switch (params->format & AFMT_I2S_MASK) {
+		case AFMT_I2S_LEFT_J:
+			i2s_fmt = I2S_FORMAT_LEFT_J;
+			break;
+		case AFMT_I2S_RIGHT_J:
+			i2s_fmt = I2S_FORMAT_RIGHT_J;
+			break;
+		default:
+			i2s_fmt = I2S_FORMAT_PHILIPS;
+			break;
+		}
+		reg_write(priv, REG_I2S_FORMAT, i2s_fmt);
 		break;
 
 	default:
@@ -992,23 +1007,15 @@ 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.format = AFMT_I2S | AFMT_I2S_PHILIPS;
+		break;
+	case HDMI_LEFT_J:
+		audio.format = AFMT_I2S | AFMT_I2S_LEFT_J;
+		break;
+	case HDMI_RIGHT_J:
+		audio.format = AFMT_I2S | AFMT_I2S_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;
 		break;
 	default:
@@ -1016,11 +1023,25 @@ static int tda998x_audio_hw_params(struct device *dev, void *data,
 		return -EINVAL;
 	}
 
+	for (i = 0; i < ARRAY_SIZE(priv->audio_port); i++)
+		if (priv->audio_port[i].format == (audio.format & AFMT_MASK))
+			audio.config = priv->audio_port[i].config;
+
 	if (audio.config == 0) {
 		dev_err(dev, "%s: No audio configuration found\n", __func__);
 		return -EINVAL;
 	}
 
+	if ((audio.format & AFMT_MASK) == HDMI_I2S &&
+	    (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);
diff --git a/include/drm/i2c/tda998x.h b/include/drm/i2c/tda998x.h
index 3cb25ccbe5e6..b0864f0be017 100644
--- a/include/drm/i2c/tda998x.h
+++ b/include/drm/i2c/tda998x.h
@@ -6,9 +6,14 @@
 #include <dt-bindings/display/tda998x.h>
 
 enum {
-	AFMT_UNUSED =	0,
-	AFMT_SPDIF =	TDA998x_SPDIF,
-	AFMT_I2S =	TDA998x_I2S,
+	AFMT_UNUSED      = 0,
+	AFMT_SPDIF       = TDA998x_SPDIF,
+	AFMT_I2S         = TDA998x_I2S,
+	AFMT_MASK        = AFMT_SPDIF | AFMT_I2S,
+	AFMT_I2S_PHILIPS = 0 << 4,
+	AFMT_I2S_LEFT_J  = 1 << 4,
+	AFMT_I2S_RIGHT_J = 2 << 4,
+	AFMT_I2S_MASK    = 3 << 4,
 };
 
 struct tda998x_audio_params {

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