[PATCH RFC 0/10] tda998x initial cleanups from bridge conversion

All of lore.kernel.org
 help / color / mirror / Atom feed

* [PATCH RFC 0/10] tda998x initial cleanups from bridge conversion
@ 2016-10-23 19:10 ` Russell King - ARM Linux
  0 siblings, 0 replies; 22+ messages in thread
From: Russell King - ARM Linux @ 2016-10-23 19:10 UTC (permalink / raw)
  To: linux-arm-kernel

As part of the discussion about converting tda998x to a bridge, here's
some preparatory work for that, which includes a bunch of fixes.  I'm
sending this out _early_ as I'm not going to be working on any kernel
stuff next week (it's likely I won't even be reading email.)  So it may
be a little rough around the edges.

Essentially, this is a series of cleanups, complexity removal, and
avoiding races with the newly introduced audio support.  Even without
the bridge conversion, I think all these are still worthwhile to have.

This series of changes can also be found in my drm-tda998x-devel branch
as an unstable series of commits (iow, I'm going to rebase/rework these
at some point, so the commit IDs are not stable, so do not merge this.)

 git://git.armlinux.org.uk/~rmk/linux-arm.git drm-tda998x-devel

 drivers/gpu/drm/i2c/tda998x_drv.c | 782 +++++++++++++++++++-------------------
 1 file changed, 394 insertions(+), 388 deletions(-)

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH RFC 0/10] tda998x initial cleanups from bridge conversion
@ 2016-10-23 19:10 ` Russell King - ARM Linux
  0 siblings, 0 replies; 22+ messages in thread
From: Russell King - ARM Linux @ 2016-10-23 19:10 UTC (permalink / raw)
  To: dri-devel, linux-arm-kernel

As part of the discussion about converting tda998x to a bridge, here's
some preparatory work for that, which includes a bunch of fixes.  I'm
sending this out _early_ as I'm not going to be working on any kernel
stuff next week (it's likely I won't even be reading email.)  So it may
be a little rough around the edges.

Essentially, this is a series of cleanups, complexity removal, and
avoiding races with the newly introduced audio support.  Even without
the bridge conversion, I think all these are still worthwhile to have.

This series of changes can also be found in my drm-tda998x-devel branch
as an unstable series of commits (iow, I'm going to rebase/rework these
at some point, so the commit IDs are not stable, so do not merge this.)

 git://git.armlinux.org.uk/~rmk/linux-arm.git drm-tda998x-devel

 drivers/gpu/drm/i2c/tda998x_drv.c | 782 +++++++++++++++++++-------------------
 1 file changed, 394 insertions(+), 388 deletions(-)

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH RFC 01/10] drm/i2c: tda998x: avoid race in tda998x_encoder_mode_set()
  2016-10-23 19:10 ` Russell King - ARM Linux
@ 2016-10-23 19:10   ` Russell King
  -1 siblings, 0 replies; 22+ messages in thread
From: Russell King @ 2016-10-23 19:10 UTC (permalink / raw)
  To: linux-arm-kernel

As priv->audio_params can now be changed at run time, we need to be more
careful about how we deal with a mode set.  We must take the audio lock
while checking if there's a valid audio configuration.

However, it's slightly worse than that - during mode set, we mute the
audio, and it must not be unmuted until we have finished the mode set.
It is possible that the audio side may start while a mode set is in
progress, so take the audio_mutex lock around the whole mode setting
procedure.

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

diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
index 9798d400d817..d72bc30a3bce 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -1074,13 +1074,12 @@ tda998x_encoder_mode_set(struct drm_encoder *encoder,
 
 		tda998x_write_avi(priv, adjusted_mode);
 
-		if (priv->audio_params.format != AFMT_UNUSED) {
-			mutex_lock(&priv->audio_mutex);
+		mutex_lock(&priv->audio_mutex);
+		if (priv->audio_params.format != AFMT_UNUSED)
 			tda998x_configure_audio(priv,
 						&priv->audio_params,
 						adjusted_mode->clock);
-			mutex_unlock(&priv->audio_mutex);
-		}
+		mutex_unlock(&priv->audio_mutex);
 	}
 }
 
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH RFC 01/10] drm/i2c: tda998x: avoid race in tda998x_encoder_mode_set()
@ 2016-10-23 19:10   ` Russell King
  0 siblings, 0 replies; 22+ messages in thread
From: Russell King @ 2016-10-23 19:10 UTC (permalink / raw)
  To: dri-devel, linux-arm-kernel

As priv->audio_params can now be changed at run time, we need to be more
careful about how we deal with a mode set.  We must take the audio lock
while checking if there's a valid audio configuration.

However, it's slightly worse than that - during mode set, we mute the
audio, and it must not be unmuted until we have finished the mode set.
It is possible that the audio side may start while a mode set is in
progress, so take the audio_mutex lock around the whole mode setting
procedure.

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

diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
index 9798d400d817..d72bc30a3bce 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -1074,13 +1074,12 @@ tda998x_encoder_mode_set(struct drm_encoder *encoder,
 
 		tda998x_write_avi(priv, adjusted_mode);
 
-		if (priv->audio_params.format != AFMT_UNUSED) {
-			mutex_lock(&priv->audio_mutex);
+		mutex_lock(&priv->audio_mutex);
+		if (priv->audio_params.format != AFMT_UNUSED)
 			tda998x_configure_audio(priv,
 						&priv->audio_params,
 						adjusted_mode->clock);
-			mutex_unlock(&priv->audio_mutex);
-		}
+		mutex_unlock(&priv->audio_mutex);
 	}
 }
 
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH RFC 02/10] drm/i2c: tda998x: avoid configuring audio for DVI mode
  2016-10-23 19:10 ` Russell King - ARM Linux
@ 2016-10-23 19:10   ` Russell King
  -1 siblings, 0 replies; 22+ messages in thread
From: Russell King @ 2016-10-23 19:10 UTC (permalink / raw)
  To: linux-arm-kernel

We must not configure the audio path when the sink is a DVI device, as
DVI has no capability to receive HDMI audio.  HDMI audio is a HDMI only
feature, requiring HDMI infoframes.

There's a question concerning how to handle a DVI connected device when
the audio device is opened - we save the audio configuration, so that if
a HDMI device is hotplugged, audio will then work.  This seems a
reasonable expectation.

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

diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
index d72bc30a3bce..495ee3fed661 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -44,6 +44,7 @@ struct tda998x_priv {
 	u8 current_page;
 	int dpms;
 	bool is_hdmi_sink;
+	bool is_hdmi_config;
 	u8 vip_cntrl_0;
 	u8 vip_cntrl_1;
 	u8 vip_cntrl_2;
@@ -971,6 +972,8 @@ tda998x_encoder_mode_set(struct drm_encoder *encoder,
 			div = 3;
 	}
 
+	mutex_lock(&priv->audio_mutex);
+
 	/* mute the audio FIFO: */
 	reg_set(priv, REG_AIP_CNTRL_0, AIP_CNTRL_0_RST_FIFO);
 
@@ -1074,13 +1077,14 @@ tda998x_encoder_mode_set(struct drm_encoder *encoder,
 
 		tda998x_write_avi(priv, adjusted_mode);
 
-		mutex_lock(&priv->audio_mutex);
 		if (priv->audio_params.format != AFMT_UNUSED)
 			tda998x_configure_audio(priv,
 						&priv->audio_params,
 						adjusted_mode->clock);
-		mutex_unlock(&priv->audio_mutex);
 	}
+
+	priv->is_hdmi_config = priv->is_hdmi_sink;
+	mutex_unlock(&priv->audio_mutex);
 }
 
 static enum drm_connector_status
@@ -1264,9 +1268,12 @@ static int tda998x_audio_hw_params(struct device *dev, void *data,
 	}
 
 	mutex_lock(&priv->audio_mutex);
-	ret = tda998x_configure_audio(priv,
-				      &audio,
-				      priv->encoder.crtc->hwmode.clock);
+	/* We must not program the TDA998x for audio if the sink is DVI. */
+	if (priv->is_hdmi_config)
+		ret = tda998x_configure_audio(priv, &audio,
+					      priv->encoder.crtc->hwmode.clock);
+	else
+		ret = 0;
 
 	if (ret == 0)
 		priv->audio_params = audio;
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH RFC 02/10] drm/i2c: tda998x: avoid configuring audio for DVI mode
@ 2016-10-23 19:10   ` Russell King
  0 siblings, 0 replies; 22+ messages in thread
From: Russell King @ 2016-10-23 19:10 UTC (permalink / raw)
  To: dri-devel, linux-arm-kernel

We must not configure the audio path when the sink is a DVI device, as
DVI has no capability to receive HDMI audio.  HDMI audio is a HDMI only
feature, requiring HDMI infoframes.

There's a question concerning how to handle a DVI connected device when
the audio device is opened - we save the audio configuration, so that if
a HDMI device is hotplugged, audio will then work.  This seems a
reasonable expectation.

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

diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
index d72bc30a3bce..495ee3fed661 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -44,6 +44,7 @@ struct tda998x_priv {
 	u8 current_page;
 	int dpms;
 	bool is_hdmi_sink;
+	bool is_hdmi_config;
 	u8 vip_cntrl_0;
 	u8 vip_cntrl_1;
 	u8 vip_cntrl_2;
@@ -971,6 +972,8 @@ tda998x_encoder_mode_set(struct drm_encoder *encoder,
 			div = 3;
 	}
 
+	mutex_lock(&priv->audio_mutex);
+
 	/* mute the audio FIFO: */
 	reg_set(priv, REG_AIP_CNTRL_0, AIP_CNTRL_0_RST_FIFO);
 
@@ -1074,13 +1077,14 @@ tda998x_encoder_mode_set(struct drm_encoder *encoder,
 
 		tda998x_write_avi(priv, adjusted_mode);
 
-		mutex_lock(&priv->audio_mutex);
 		if (priv->audio_params.format != AFMT_UNUSED)
 			tda998x_configure_audio(priv,
 						&priv->audio_params,
 						adjusted_mode->clock);
-		mutex_unlock(&priv->audio_mutex);
 	}
+
+	priv->is_hdmi_config = priv->is_hdmi_sink;
+	mutex_unlock(&priv->audio_mutex);
 }
 
 static enum drm_connector_status
@@ -1264,9 +1268,12 @@ static int tda998x_audio_hw_params(struct device *dev, void *data,
 	}
 
 	mutex_lock(&priv->audio_mutex);
-	ret = tda998x_configure_audio(priv,
-				      &audio,
-				      priv->encoder.crtc->hwmode.clock);
+	/* We must not program the TDA998x for audio if the sink is DVI. */
+	if (priv->is_hdmi_config)
+		ret = tda998x_configure_audio(priv, &audio,
+					      priv->encoder.crtc->hwmode.clock);
+	else
+		ret = 0;
 
 	if (ret == 0)
 		priv->audio_params = audio;
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH RFC 03/10] drm/i2c: tda998x: avoid racy access to mode clock
  2016-10-23 19:10 ` Russell King - ARM Linux
@ 2016-10-23 19:10   ` Russell King
  -1 siblings, 0 replies; 22+ messages in thread
From: Russell King @ 2016-10-23 19:10 UTC (permalink / raw)
  To: linux-arm-kernel

Avoid a racy access to the mode clock by storing the current mode clock
during a mode set under the audio mutex.  This allows us to access it
from the audio path in a safe way.

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

diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
index 495ee3fed661..858237f7f4d6 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -48,6 +48,7 @@ struct tda998x_priv {
 	u8 vip_cntrl_0;
 	u8 vip_cntrl_1;
 	u8 vip_cntrl_2;
+	unsigned long tdms_clock;
 	struct tda998x_audio_params audio_params;
 
 	struct platform_device *audio_pdev;
@@ -714,8 +715,7 @@ 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,
-			unsigned mode_clock)
+			struct tda998x_audio_params *params)
 {
 	u8 buf[6], clksel_aip, clksel_fs, cts_n, adiv;
 	u32 n;
@@ -772,7 +772,7 @@ tda998x_configure_audio(struct tda998x_priv *priv,
 	 * assume 100MHz requires larger divider.
 	 */
 	adiv = AUDIO_DIV_SERCLK_8;
-	if (mode_clock > 100000)
+	if (priv->tdms_clock > 100000)
 		adiv++;			/* AUDIO_DIV_SERCLK_16 */
 
 	/* S/PDIF asks for a larger divider */
@@ -1077,10 +1077,10 @@ tda998x_encoder_mode_set(struct drm_encoder *encoder,
 
 		tda998x_write_avi(priv, adjusted_mode);
 
+		priv->tdms_clock = adjusted_mode->clock;
+
 		if (priv->audio_params.format != AFMT_UNUSED)
-			tda998x_configure_audio(priv,
-						&priv->audio_params,
-						adjusted_mode->clock);
+			tda998x_configure_audio(priv, &priv->audio_params);
 	}
 
 	priv->is_hdmi_config = priv->is_hdmi_sink;
@@ -1230,9 +1230,6 @@ static int tda998x_audio_hw_params(struct device *dev, void *data,
 		.cea = params->cea,
 	};
 
-	if (!priv->encoder.crtc)
-		return -ENODEV;
-
 	memcpy(audio.status, params->iec.status,
 	       min(sizeof(audio.status), sizeof(params->iec.status)));
 
@@ -1270,8 +1267,7 @@ static int tda998x_audio_hw_params(struct device *dev, void *data,
 	mutex_lock(&priv->audio_mutex);
 	/* We must not program the TDA998x for audio if the sink is DVI. */
 	if (priv->is_hdmi_config)
-		ret = tda998x_configure_audio(priv, &audio,
-					      priv->encoder.crtc->hwmode.clock);
+		ret = tda998x_configure_audio(priv, &audio);
 	else
 		ret = 0;
 
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH RFC 03/10] drm/i2c: tda998x: avoid racy access to mode clock
@ 2016-10-23 19:10   ` Russell King
  0 siblings, 0 replies; 22+ messages in thread
From: Russell King @ 2016-10-23 19:10 UTC (permalink / raw)
  To: dri-devel, linux-arm-kernel

Avoid a racy access to the mode clock by storing the current mode clock
during a mode set under the audio mutex.  This allows us to access it
from the audio path in a safe way.

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

diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
index 495ee3fed661..858237f7f4d6 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -48,6 +48,7 @@ struct tda998x_priv {
 	u8 vip_cntrl_0;
 	u8 vip_cntrl_1;
 	u8 vip_cntrl_2;
+	unsigned long tdms_clock;
 	struct tda998x_audio_params audio_params;
 
 	struct platform_device *audio_pdev;
@@ -714,8 +715,7 @@ 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,
-			unsigned mode_clock)
+			struct tda998x_audio_params *params)
 {
 	u8 buf[6], clksel_aip, clksel_fs, cts_n, adiv;
 	u32 n;
@@ -772,7 +772,7 @@ tda998x_configure_audio(struct tda998x_priv *priv,
 	 * assume 100MHz requires larger divider.
 	 */
 	adiv = AUDIO_DIV_SERCLK_8;
-	if (mode_clock > 100000)
+	if (priv->tdms_clock > 100000)
 		adiv++;			/* AUDIO_DIV_SERCLK_16 */
 
 	/* S/PDIF asks for a larger divider */
@@ -1077,10 +1077,10 @@ tda998x_encoder_mode_set(struct drm_encoder *encoder,
 
 		tda998x_write_avi(priv, adjusted_mode);
 
+		priv->tdms_clock = adjusted_mode->clock;
+
 		if (priv->audio_params.format != AFMT_UNUSED)
-			tda998x_configure_audio(priv,
-						&priv->audio_params,
-						adjusted_mode->clock);
+			tda998x_configure_audio(priv, &priv->audio_params);
 	}
 
 	priv->is_hdmi_config = priv->is_hdmi_sink;
@@ -1230,9 +1230,6 @@ static int tda998x_audio_hw_params(struct device *dev, void *data,
 		.cea = params->cea,
 	};
 
-	if (!priv->encoder.crtc)
-		return -ENODEV;
-
 	memcpy(audio.status, params->iec.status,
 	       min(sizeof(audio.status), sizeof(params->iec.status)));
 
@@ -1270,8 +1267,7 @@ static int tda998x_audio_hw_params(struct device *dev, void *data,
 	mutex_lock(&priv->audio_mutex);
 	/* We must not program the TDA998x for audio if the sink is DVI. */
 	if (priv->is_hdmi_config)
-		ret = tda998x_configure_audio(priv, &audio,
-					      priv->encoder.crtc->hwmode.clock);
+		ret = tda998x_configure_audio(priv, &audio);
 	else
 		ret = 0;
 
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH RFC 04/10] drm/i2c: tda998x: correct function name in comments
  2016-10-23 19:10 ` Russell King - ARM Linux
@ 2016-10-23 19:10   ` Russell King
  -1 siblings, 0 replies; 22+ messages in thread
From: Russell King @ 2016-10-23 19:10 UTC (permalink / raw)
  To: linux-arm-kernel

Correct two references to tda998x_connector_get_modes() which were
incorrectly referring to tda998x_encoder_get_modes().

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

diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
index 858237f7f4d6..134cd3f26a07 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -581,9 +581,9 @@ tda998x_reset(struct tda998x_priv *priv)
  * HPD assertion: it needs a delay of 100ms to avoid timing out while
  * trying to read EDID data.
  *
- * However, tda998x_encoder_get_modes() may be called at any moment
+ * However, tda998x_connector_get_modes() may be called at any moment
  * after tda998x_connector_detect() indicates that we are connected, so
- * we need to delay probing modes in tda998x_encoder_get_modes() after
+ * we need to delay probing modes in tda998x_connector_get_modes() after
  * we have seen a HPD inactive->active transition.  This code implements
  * that delay.
  */
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH RFC 04/10] drm/i2c: tda998x: correct function name in comments
@ 2016-10-23 19:10   ` Russell King
  0 siblings, 0 replies; 22+ messages in thread
From: Russell King @ 2016-10-23 19:10 UTC (permalink / raw)
  To: dri-devel, linux-arm-kernel

Correct two references to tda998x_connector_get_modes() which were
incorrectly referring to tda998x_encoder_get_modes().

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

diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
index 858237f7f4d6..134cd3f26a07 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -581,9 +581,9 @@ tda998x_reset(struct tda998x_priv *priv)
  * HPD assertion: it needs a delay of 100ms to avoid timing out while
  * trying to read EDID data.
  *
- * However, tda998x_encoder_get_modes() may be called at any moment
+ * However, tda998x_connector_get_modes() may be called at any moment
  * after tda998x_connector_detect() indicates that we are connected, so
- * we need to delay probing modes in tda998x_encoder_get_modes() after
+ * we need to delay probing modes in tda998x_connector_get_modes() after
  * we have seen a HPD inactive->active transition.  This code implements
  * that delay.
  */
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH RFC 05/10] drm/i2c: tda998x: move and rename tda998x_encoder_set_config()
  2016-10-23 19:10 ` Russell King - ARM Linux
@ 2016-10-23 19:10   ` Russell King
  -1 siblings, 0 replies; 22+ messages in thread
From: Russell King @ 2016-10-23 19:10 UTC (permalink / raw)
  To: linux-arm-kernel

The naming of tda998x_encoder_set_config() is a left-over from when
TDA998x was a slave encoder.  Since this is part of the initialisation,
drop the _encoder from the name, and move it near tda998x_bind().

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

diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
index 134cd3f26a07..d3951aee2b09 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -822,25 +822,6 @@ tda998x_configure_audio(struct tda998x_priv *priv,
 
 /* DRM encoder functions */
 
-static void tda998x_encoder_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) |
-			    VIP_CNTRL_0_SWAP_B(p->swap_b) |
-			    (p->mirr_b ? VIP_CNTRL_0_MIRR_B : 0);
-	priv->vip_cntrl_1 = VIP_CNTRL_1_SWAP_C(p->swap_c) |
-			    (p->mirr_c ? VIP_CNTRL_1_MIRR_C : 0) |
-			    VIP_CNTRL_1_SWAP_D(p->swap_d) |
-			    (p->mirr_d ? VIP_CNTRL_1_MIRR_D : 0);
-	priv->vip_cntrl_2 = VIP_CNTRL_2_SWAP_E(p->swap_e) |
-			    (p->mirr_e ? VIP_CNTRL_2_MIRR_E : 0) |
-			    VIP_CNTRL_2_SWAP_F(p->swap_f) |
-			    (p->mirr_f ? VIP_CNTRL_2_MIRR_F : 0);
-
-	priv->audio_params = p->audio_params;
-}
-
 static void tda998x_encoder_dpms(struct drm_encoder *encoder, int mode)
 {
 	struct tda998x_priv *priv = enc_to_tda998x_priv(encoder);
@@ -1608,6 +1589,25 @@ static const struct drm_connector_funcs tda998x_connector_funcs = {
 	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
 };
 
+static void 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) |
+			    VIP_CNTRL_0_SWAP_B(p->swap_b) |
+			    (p->mirr_b ? VIP_CNTRL_0_MIRR_B : 0);
+	priv->vip_cntrl_1 = VIP_CNTRL_1_SWAP_C(p->swap_c) |
+			    (p->mirr_c ? VIP_CNTRL_1_MIRR_C : 0) |
+			    VIP_CNTRL_1_SWAP_D(p->swap_d) |
+			    (p->mirr_d ? VIP_CNTRL_1_MIRR_D : 0);
+	priv->vip_cntrl_2 = VIP_CNTRL_2_SWAP_E(p->swap_e) |
+			    (p->mirr_e ? VIP_CNTRL_2_MIRR_E : 0) |
+			    VIP_CNTRL_2_SWAP_F(p->swap_f) |
+			    (p->mirr_f ? VIP_CNTRL_2_MIRR_F : 0);
+
+	priv->audio_params = p->audio_params;
+}
+
 static int tda998x_bind(struct device *dev, struct device *master, void *data)
 {
 	struct tda998x_encoder_params *params = dev->platform_data;
@@ -1640,7 +1640,7 @@ static int tda998x_bind(struct device *dev, struct device *master, void *data)
 		return ret;
 
 	if (!dev->of_node && params)
-		tda998x_encoder_set_config(priv, params);
+		tda998x_set_config(priv, params);
 
 	tda998x_encoder_set_polling(priv, &priv->connector);
 
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH RFC 05/10] drm/i2c: tda998x: move and rename tda998x_encoder_set_config()
@ 2016-10-23 19:10   ` Russell King
  0 siblings, 0 replies; 22+ messages in thread
From: Russell King @ 2016-10-23 19:10 UTC (permalink / raw)
  To: dri-devel, linux-arm-kernel

The naming of tda998x_encoder_set_config() is a left-over from when
TDA998x was a slave encoder.  Since this is part of the initialisation,
drop the _encoder from the name, and move it near tda998x_bind().

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

diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
index 134cd3f26a07..d3951aee2b09 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -822,25 +822,6 @@ tda998x_configure_audio(struct tda998x_priv *priv,
 
 /* DRM encoder functions */
 
-static void tda998x_encoder_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) |
-			    VIP_CNTRL_0_SWAP_B(p->swap_b) |
-			    (p->mirr_b ? VIP_CNTRL_0_MIRR_B : 0);
-	priv->vip_cntrl_1 = VIP_CNTRL_1_SWAP_C(p->swap_c) |
-			    (p->mirr_c ? VIP_CNTRL_1_MIRR_C : 0) |
-			    VIP_CNTRL_1_SWAP_D(p->swap_d) |
-			    (p->mirr_d ? VIP_CNTRL_1_MIRR_D : 0);
-	priv->vip_cntrl_2 = VIP_CNTRL_2_SWAP_E(p->swap_e) |
-			    (p->mirr_e ? VIP_CNTRL_2_MIRR_E : 0) |
-			    VIP_CNTRL_2_SWAP_F(p->swap_f) |
-			    (p->mirr_f ? VIP_CNTRL_2_MIRR_F : 0);
-
-	priv->audio_params = p->audio_params;
-}
-
 static void tda998x_encoder_dpms(struct drm_encoder *encoder, int mode)
 {
 	struct tda998x_priv *priv = enc_to_tda998x_priv(encoder);
@@ -1608,6 +1589,25 @@ static const struct drm_connector_funcs tda998x_connector_funcs = {
 	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
 };
 
+static void 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) |
+			    VIP_CNTRL_0_SWAP_B(p->swap_b) |
+			    (p->mirr_b ? VIP_CNTRL_0_MIRR_B : 0);
+	priv->vip_cntrl_1 = VIP_CNTRL_1_SWAP_C(p->swap_c) |
+			    (p->mirr_c ? VIP_CNTRL_1_MIRR_C : 0) |
+			    VIP_CNTRL_1_SWAP_D(p->swap_d) |
+			    (p->mirr_d ? VIP_CNTRL_1_MIRR_D : 0);
+	priv->vip_cntrl_2 = VIP_CNTRL_2_SWAP_E(p->swap_e) |
+			    (p->mirr_e ? VIP_CNTRL_2_MIRR_E : 0) |
+			    VIP_CNTRL_2_SWAP_F(p->swap_f) |
+			    (p->mirr_f ? VIP_CNTRL_2_MIRR_F : 0);
+
+	priv->audio_params = p->audio_params;
+}
+
 static int tda998x_bind(struct device *dev, struct device *master, void *data)
 {
 	struct tda998x_encoder_params *params = dev->platform_data;
@@ -1640,7 +1640,7 @@ static int tda998x_bind(struct device *dev, struct device *master, void *data)
 		return ret;
 
 	if (!dev->of_node && params)
-		tda998x_encoder_set_config(priv, params);
+		tda998x_set_config(priv, params);
 
 	tda998x_encoder_set_polling(priv, &priv->connector);
 
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH RFC 06/10] drm/i2c: tda998x: group connector functions and funcs together
  2016-10-23 19:10 ` Russell King - ARM Linux
@ 2016-10-23 19:10   ` Russell King
  -1 siblings, 0 replies; 22+ messages in thread
From: Russell King @ 2016-10-23 19:10 UTC (permalink / raw)
  To: linux-arm-kernel

Group the TDA998x connector functions and funcs structures together
before the encoder support, rather than scattered amongst the rest of
the file.  This keeps like code together.

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

diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
index d3951aee2b09..bb5389fbd059 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -820,6 +820,165 @@ tda998x_configure_audio(struct tda998x_priv *priv,
 	return tda998x_write_aif(priv, &params->cea);
 }
 
+/* DRM connector functions */
+
+static int tda998x_connector_dpms(struct drm_connector *connector, int mode)
+{
+	if (drm_core_check_feature(connector->dev, DRIVER_ATOMIC))
+		return drm_atomic_helper_connector_dpms(connector, mode);
+	else
+		return drm_helper_connector_dpms(connector, mode);
+}
+
+static enum drm_connector_status
+tda998x_connector_detect(struct drm_connector *connector, bool force)
+{
+	struct tda998x_priv *priv = conn_to_tda998x_priv(connector);
+	u8 val = cec_read(priv, REG_CEC_RXSHPDLEV);
+
+	return (val & CEC_RXSHPDLEV_HPD) ? connector_status_connected :
+			connector_status_disconnected;
+}
+
+static void tda998x_connector_destroy(struct drm_connector *connector)
+{
+	drm_connector_unregister(connector);
+	drm_connector_cleanup(connector);
+}
+
+static const struct drm_connector_funcs tda998x_connector_funcs = {
+	.dpms = tda998x_connector_dpms,
+	.reset = drm_atomic_helper_connector_reset,
+	.fill_modes = drm_helper_probe_single_connector_modes,
+	.detect = tda998x_connector_detect,
+	.destroy = tda998x_connector_destroy,
+	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
+	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
+};
+
+static int read_edid_block(void *data, u8 *buf, unsigned int blk, size_t length)
+{
+	struct tda998x_priv *priv = data;
+	u8 offset, segptr;
+	int ret, i;
+
+	offset = (blk & 1) ? 128 : 0;
+	segptr = blk / 2;
+
+	reg_write(priv, REG_DDC_ADDR, 0xa0);
+	reg_write(priv, REG_DDC_OFFS, offset);
+	reg_write(priv, REG_DDC_SEGM_ADDR, 0x60);
+	reg_write(priv, REG_DDC_SEGM, segptr);
+
+	/* enable reading EDID: */
+	priv->wq_edid_wait = 1;
+	reg_write(priv, REG_EDID_CTRL, 0x1);
+
+	/* flag must be cleared by sw: */
+	reg_write(priv, REG_EDID_CTRL, 0x0);
+
+	/* wait for block read to complete: */
+	if (priv->hdmi->irq) {
+		i = wait_event_timeout(priv->wq_edid,
+					!priv->wq_edid_wait,
+					msecs_to_jiffies(100));
+		if (i < 0) {
+			dev_err(&priv->hdmi->dev, "read edid wait err %d\n", i);
+			return i;
+		}
+	} else {
+		for (i = 100; i > 0; i--) {
+			msleep(1);
+			ret = reg_read(priv, REG_INT_FLAGS_2);
+			if (ret < 0)
+				return ret;
+			if (ret & INT_FLAGS_2_EDID_BLK_RD)
+				break;
+		}
+	}
+
+	if (i == 0) {
+		dev_err(&priv->hdmi->dev, "read edid timeout\n");
+		return -ETIMEDOUT;
+	}
+
+	ret = reg_read_range(priv, REG_EDID_DATA_0, buf, length);
+	if (ret != length) {
+		dev_err(&priv->hdmi->dev, "failed to read edid block %d: %d\n",
+			blk, ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int tda998x_connector_get_modes(struct drm_connector *connector)
+{
+	struct tda998x_priv *priv = conn_to_tda998x_priv(connector);
+	struct edid *edid;
+	int n;
+
+	/*
+	 * If we get killed while waiting for the HPD timeout, return
+	 * no modes found: we are not in a restartable path, so we
+	 * can't handle signals gracefully.
+	 */
+	if (tda998x_edid_delay_wait(priv))
+		return 0;
+
+	if (priv->rev == TDA19988)
+		reg_clear(priv, REG_TX4, TX4_PD_RAM);
+
+	edid = drm_do_get_edid(connector, read_edid_block, priv);
+
+	if (priv->rev == TDA19988)
+		reg_set(priv, REG_TX4, TX4_PD_RAM);
+
+	if (!edid) {
+		dev_warn(&priv->hdmi->dev, "failed to read EDID\n");
+		return 0;
+	}
+
+	drm_mode_connector_update_edid_property(connector, edid);
+	n = drm_add_edid_modes(connector, edid);
+	priv->is_hdmi_sink = drm_detect_hdmi_monitor(edid);
+	drm_edid_to_eld(connector, edid);
+
+	kfree(edid);
+
+	return n;
+}
+
+static int tda998x_connector_mode_valid(struct drm_connector *connector,
+					struct drm_display_mode *mode)
+{
+	/* TDA19988 dotclock can go up to 165MHz */
+	struct tda998x_priv *priv = conn_to_tda998x_priv(connector);
+
+	if (mode->clock > ((priv->rev == TDA19988) ? 165000 : 150000))
+		return MODE_CLOCK_HIGH;
+	if (mode->htotal >= BIT(13))
+		return MODE_BAD_HVALUE;
+	if (mode->vtotal >= BIT(11))
+		return MODE_BAD_VVALUE;
+	return MODE_OK;
+}
+
+static struct drm_encoder *
+tda998x_connector_best_encoder(struct drm_connector *connector)
+{
+	struct tda998x_priv *priv = conn_to_tda998x_priv(connector);
+
+	return &priv->encoder;
+}
+
+static
+const struct drm_connector_helper_funcs tda998x_connector_helper_funcs = {
+	.get_modes = tda998x_connector_get_modes,
+	.mode_valid = tda998x_connector_mode_valid,
+	.best_encoder = tda998x_connector_best_encoder,
+};
+
 /* DRM encoder functions */
 
 static void tda998x_encoder_dpms(struct drm_encoder *encoder, int mode)
@@ -855,21 +1014,6 @@ static void tda998x_encoder_dpms(struct drm_encoder *encoder, int mode)
 	priv->dpms = mode;
 }
 
-static int tda998x_connector_mode_valid(struct drm_connector *connector,
-					struct drm_display_mode *mode)
-{
-	/* TDA19988 dotclock can go up to 165MHz */
-	struct tda998x_priv *priv = conn_to_tda998x_priv(connector);
-
-	if (mode->clock > ((priv->rev == TDA19988) ? 165000 : 150000))
-		return MODE_CLOCK_HIGH;
-	if (mode->htotal >= BIT(13))
-		return MODE_BAD_HVALUE;
-	if (mode->vtotal >= BIT(11))
-		return MODE_BAD_VVALUE;
-	return MODE_OK;
-}
-
 static void
 tda998x_encoder_mode_set(struct drm_encoder *encoder,
 			 struct drm_display_mode *mode,
@@ -1068,109 +1212,6 @@ tda998x_encoder_mode_set(struct drm_encoder *encoder,
 	mutex_unlock(&priv->audio_mutex);
 }
 
-static enum drm_connector_status
-tda998x_connector_detect(struct drm_connector *connector, bool force)
-{
-	struct tda998x_priv *priv = conn_to_tda998x_priv(connector);
-	u8 val = cec_read(priv, REG_CEC_RXSHPDLEV);
-
-	return (val & CEC_RXSHPDLEV_HPD) ? connector_status_connected :
-			connector_status_disconnected;
-}
-
-static int read_edid_block(void *data, u8 *buf, unsigned int blk, size_t length)
-{
-	struct tda998x_priv *priv = data;
-	u8 offset, segptr;
-	int ret, i;
-
-	offset = (blk & 1) ? 128 : 0;
-	segptr = blk / 2;
-
-	reg_write(priv, REG_DDC_ADDR, 0xa0);
-	reg_write(priv, REG_DDC_OFFS, offset);
-	reg_write(priv, REG_DDC_SEGM_ADDR, 0x60);
-	reg_write(priv, REG_DDC_SEGM, segptr);
-
-	/* enable reading EDID: */
-	priv->wq_edid_wait = 1;
-	reg_write(priv, REG_EDID_CTRL, 0x1);
-
-	/* flag must be cleared by sw: */
-	reg_write(priv, REG_EDID_CTRL, 0x0);
-
-	/* wait for block read to complete: */
-	if (priv->hdmi->irq) {
-		i = wait_event_timeout(priv->wq_edid,
-					!priv->wq_edid_wait,
-					msecs_to_jiffies(100));
-		if (i < 0) {
-			dev_err(&priv->hdmi->dev, "read edid wait err %d\n", i);
-			return i;
-		}
-	} else {
-		for (i = 100; i > 0; i--) {
-			msleep(1);
-			ret = reg_read(priv, REG_INT_FLAGS_2);
-			if (ret < 0)
-				return ret;
-			if (ret & INT_FLAGS_2_EDID_BLK_RD)
-				break;
-		}
-	}
-
-	if (i == 0) {
-		dev_err(&priv->hdmi->dev, "read edid timeout\n");
-		return -ETIMEDOUT;
-	}
-
-	ret = reg_read_range(priv, REG_EDID_DATA_0, buf, length);
-	if (ret != length) {
-		dev_err(&priv->hdmi->dev, "failed to read edid block %d: %d\n",
-			blk, ret);
-		return ret;
-	}
-
-	return 0;
-}
-
-static int tda998x_connector_get_modes(struct drm_connector *connector)
-{
-	struct tda998x_priv *priv = conn_to_tda998x_priv(connector);
-	struct edid *edid;
-	int n;
-
-	/*
-	 * If we get killed while waiting for the HPD timeout, return
-	 * no modes found: we are not in a restartable path, so we
-	 * can't handle signals gracefully.
-	 */
-	if (tda998x_edid_delay_wait(priv))
-		return 0;
-
-	if (priv->rev == TDA19988)
-		reg_clear(priv, REG_TX4, TX4_PD_RAM);
-
-	edid = drm_do_get_edid(connector, read_edid_block, priv);
-
-	if (priv->rev == TDA19988)
-		reg_set(priv, REG_TX4, TX4_PD_RAM);
-
-	if (!edid) {
-		dev_warn(&priv->hdmi->dev, "failed to read EDID\n");
-		return 0;
-	}
-
-	drm_mode_connector_update_edid_property(connector, edid);
-	n = drm_add_edid_modes(connector, edid);
-	priv->is_hdmi_sink = drm_detect_hdmi_monitor(edid);
-	drm_edid_to_eld(connector, edid);
-
-	kfree(edid);
-
-	return n;
-}
-
 static void tda998x_encoder_set_polling(struct tda998x_priv *priv,
 					struct drm_connector *connector)
 {
@@ -1550,45 +1591,6 @@ static const struct drm_encoder_funcs tda998x_encoder_funcs = {
 	.destroy = tda998x_encoder_destroy,
 };
 
-static struct drm_encoder *
-tda998x_connector_best_encoder(struct drm_connector *connector)
-{
-	struct tda998x_priv *priv = conn_to_tda998x_priv(connector);
-
-	return &priv->encoder;
-}
-
-static
-const struct drm_connector_helper_funcs tda998x_connector_helper_funcs = {
-	.get_modes = tda998x_connector_get_modes,
-	.mode_valid = tda998x_connector_mode_valid,
-	.best_encoder = tda998x_connector_best_encoder,
-};
-
-static void tda998x_connector_destroy(struct drm_connector *connector)
-{
-	drm_connector_unregister(connector);
-	drm_connector_cleanup(connector);
-}
-
-static int tda998x_connector_dpms(struct drm_connector *connector, int mode)
-{
-	if (drm_core_check_feature(connector->dev, DRIVER_ATOMIC))
-		return drm_atomic_helper_connector_dpms(connector, mode);
-	else
-		return drm_helper_connector_dpms(connector, mode);
-}
-
-static const struct drm_connector_funcs tda998x_connector_funcs = {
-	.dpms = tda998x_connector_dpms,
-	.reset = drm_atomic_helper_connector_reset,
-	.fill_modes = drm_helper_probe_single_connector_modes,
-	.detect = tda998x_connector_detect,
-	.destroy = tda998x_connector_destroy,
-	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
-	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
-};
-
 static void tda998x_set_config(struct tda998x_priv *priv,
 			       const struct tda998x_encoder_params *p)
 {
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH RFC 06/10] drm/i2c: tda998x: group connector functions and funcs together
@ 2016-10-23 19:10   ` Russell King
  0 siblings, 0 replies; 22+ messages in thread
From: Russell King @ 2016-10-23 19:10 UTC (permalink / raw)
  To: dri-devel, linux-arm-kernel

Group the TDA998x connector functions and funcs structures together
before the encoder support, rather than scattered amongst the rest of
the file.  This keeps like code together.

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

diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
index d3951aee2b09..bb5389fbd059 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -820,6 +820,165 @@ tda998x_configure_audio(struct tda998x_priv *priv,
 	return tda998x_write_aif(priv, &params->cea);
 }
 
+/* DRM connector functions */
+
+static int tda998x_connector_dpms(struct drm_connector *connector, int mode)
+{
+	if (drm_core_check_feature(connector->dev, DRIVER_ATOMIC))
+		return drm_atomic_helper_connector_dpms(connector, mode);
+	else
+		return drm_helper_connector_dpms(connector, mode);
+}
+
+static enum drm_connector_status
+tda998x_connector_detect(struct drm_connector *connector, bool force)
+{
+	struct tda998x_priv *priv = conn_to_tda998x_priv(connector);
+	u8 val = cec_read(priv, REG_CEC_RXSHPDLEV);
+
+	return (val & CEC_RXSHPDLEV_HPD) ? connector_status_connected :
+			connector_status_disconnected;
+}
+
+static void tda998x_connector_destroy(struct drm_connector *connector)
+{
+	drm_connector_unregister(connector);
+	drm_connector_cleanup(connector);
+}
+
+static const struct drm_connector_funcs tda998x_connector_funcs = {
+	.dpms = tda998x_connector_dpms,
+	.reset = drm_atomic_helper_connector_reset,
+	.fill_modes = drm_helper_probe_single_connector_modes,
+	.detect = tda998x_connector_detect,
+	.destroy = tda998x_connector_destroy,
+	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
+	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
+};
+
+static int read_edid_block(void *data, u8 *buf, unsigned int blk, size_t length)
+{
+	struct tda998x_priv *priv = data;
+	u8 offset, segptr;
+	int ret, i;
+
+	offset = (blk & 1) ? 128 : 0;
+	segptr = blk / 2;
+
+	reg_write(priv, REG_DDC_ADDR, 0xa0);
+	reg_write(priv, REG_DDC_OFFS, offset);
+	reg_write(priv, REG_DDC_SEGM_ADDR, 0x60);
+	reg_write(priv, REG_DDC_SEGM, segptr);
+
+	/* enable reading EDID: */
+	priv->wq_edid_wait = 1;
+	reg_write(priv, REG_EDID_CTRL, 0x1);
+
+	/* flag must be cleared by sw: */
+	reg_write(priv, REG_EDID_CTRL, 0x0);
+
+	/* wait for block read to complete: */
+	if (priv->hdmi->irq) {
+		i = wait_event_timeout(priv->wq_edid,
+					!priv->wq_edid_wait,
+					msecs_to_jiffies(100));
+		if (i < 0) {
+			dev_err(&priv->hdmi->dev, "read edid wait err %d\n", i);
+			return i;
+		}
+	} else {
+		for (i = 100; i > 0; i--) {
+			msleep(1);
+			ret = reg_read(priv, REG_INT_FLAGS_2);
+			if (ret < 0)
+				return ret;
+			if (ret & INT_FLAGS_2_EDID_BLK_RD)
+				break;
+		}
+	}
+
+	if (i == 0) {
+		dev_err(&priv->hdmi->dev, "read edid timeout\n");
+		return -ETIMEDOUT;
+	}
+
+	ret = reg_read_range(priv, REG_EDID_DATA_0, buf, length);
+	if (ret != length) {
+		dev_err(&priv->hdmi->dev, "failed to read edid block %d: %d\n",
+			blk, ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int tda998x_connector_get_modes(struct drm_connector *connector)
+{
+	struct tda998x_priv *priv = conn_to_tda998x_priv(connector);
+	struct edid *edid;
+	int n;
+
+	/*
+	 * If we get killed while waiting for the HPD timeout, return
+	 * no modes found: we are not in a restartable path, so we
+	 * can't handle signals gracefully.
+	 */
+	if (tda998x_edid_delay_wait(priv))
+		return 0;
+
+	if (priv->rev == TDA19988)
+		reg_clear(priv, REG_TX4, TX4_PD_RAM);
+
+	edid = drm_do_get_edid(connector, read_edid_block, priv);
+
+	if (priv->rev == TDA19988)
+		reg_set(priv, REG_TX4, TX4_PD_RAM);
+
+	if (!edid) {
+		dev_warn(&priv->hdmi->dev, "failed to read EDID\n");
+		return 0;
+	}
+
+	drm_mode_connector_update_edid_property(connector, edid);
+	n = drm_add_edid_modes(connector, edid);
+	priv->is_hdmi_sink = drm_detect_hdmi_monitor(edid);
+	drm_edid_to_eld(connector, edid);
+
+	kfree(edid);
+
+	return n;
+}
+
+static int tda998x_connector_mode_valid(struct drm_connector *connector,
+					struct drm_display_mode *mode)
+{
+	/* TDA19988 dotclock can go up to 165MHz */
+	struct tda998x_priv *priv = conn_to_tda998x_priv(connector);
+
+	if (mode->clock > ((priv->rev == TDA19988) ? 165000 : 150000))
+		return MODE_CLOCK_HIGH;
+	if (mode->htotal >= BIT(13))
+		return MODE_BAD_HVALUE;
+	if (mode->vtotal >= BIT(11))
+		return MODE_BAD_VVALUE;
+	return MODE_OK;
+}
+
+static struct drm_encoder *
+tda998x_connector_best_encoder(struct drm_connector *connector)
+{
+	struct tda998x_priv *priv = conn_to_tda998x_priv(connector);
+
+	return &priv->encoder;
+}
+
+static
+const struct drm_connector_helper_funcs tda998x_connector_helper_funcs = {
+	.get_modes = tda998x_connector_get_modes,
+	.mode_valid = tda998x_connector_mode_valid,
+	.best_encoder = tda998x_connector_best_encoder,
+};
+
 /* DRM encoder functions */
 
 static void tda998x_encoder_dpms(struct drm_encoder *encoder, int mode)
@@ -855,21 +1014,6 @@ static void tda998x_encoder_dpms(struct drm_encoder *encoder, int mode)
 	priv->dpms = mode;
 }
 
-static int tda998x_connector_mode_valid(struct drm_connector *connector,
-					struct drm_display_mode *mode)
-{
-	/* TDA19988 dotclock can go up to 165MHz */
-	struct tda998x_priv *priv = conn_to_tda998x_priv(connector);
-
-	if (mode->clock > ((priv->rev == TDA19988) ? 165000 : 150000))
-		return MODE_CLOCK_HIGH;
-	if (mode->htotal >= BIT(13))
-		return MODE_BAD_HVALUE;
-	if (mode->vtotal >= BIT(11))
-		return MODE_BAD_VVALUE;
-	return MODE_OK;
-}
-
 static void
 tda998x_encoder_mode_set(struct drm_encoder *encoder,
 			 struct drm_display_mode *mode,
@@ -1068,109 +1212,6 @@ tda998x_encoder_mode_set(struct drm_encoder *encoder,
 	mutex_unlock(&priv->audio_mutex);
 }
 
-static enum drm_connector_status
-tda998x_connector_detect(struct drm_connector *connector, bool force)
-{
-	struct tda998x_priv *priv = conn_to_tda998x_priv(connector);
-	u8 val = cec_read(priv, REG_CEC_RXSHPDLEV);
-
-	return (val & CEC_RXSHPDLEV_HPD) ? connector_status_connected :
-			connector_status_disconnected;
-}
-
-static int read_edid_block(void *data, u8 *buf, unsigned int blk, size_t length)
-{
-	struct tda998x_priv *priv = data;
-	u8 offset, segptr;
-	int ret, i;
-
-	offset = (blk & 1) ? 128 : 0;
-	segptr = blk / 2;
-
-	reg_write(priv, REG_DDC_ADDR, 0xa0);
-	reg_write(priv, REG_DDC_OFFS, offset);
-	reg_write(priv, REG_DDC_SEGM_ADDR, 0x60);
-	reg_write(priv, REG_DDC_SEGM, segptr);
-
-	/* enable reading EDID: */
-	priv->wq_edid_wait = 1;
-	reg_write(priv, REG_EDID_CTRL, 0x1);
-
-	/* flag must be cleared by sw: */
-	reg_write(priv, REG_EDID_CTRL, 0x0);
-
-	/* wait for block read to complete: */
-	if (priv->hdmi->irq) {
-		i = wait_event_timeout(priv->wq_edid,
-					!priv->wq_edid_wait,
-					msecs_to_jiffies(100));
-		if (i < 0) {
-			dev_err(&priv->hdmi->dev, "read edid wait err %d\n", i);
-			return i;
-		}
-	} else {
-		for (i = 100; i > 0; i--) {
-			msleep(1);
-			ret = reg_read(priv, REG_INT_FLAGS_2);
-			if (ret < 0)
-				return ret;
-			if (ret & INT_FLAGS_2_EDID_BLK_RD)
-				break;
-		}
-	}
-
-	if (i == 0) {
-		dev_err(&priv->hdmi->dev, "read edid timeout\n");
-		return -ETIMEDOUT;
-	}
-
-	ret = reg_read_range(priv, REG_EDID_DATA_0, buf, length);
-	if (ret != length) {
-		dev_err(&priv->hdmi->dev, "failed to read edid block %d: %d\n",
-			blk, ret);
-		return ret;
-	}
-
-	return 0;
-}
-
-static int tda998x_connector_get_modes(struct drm_connector *connector)
-{
-	struct tda998x_priv *priv = conn_to_tda998x_priv(connector);
-	struct edid *edid;
-	int n;
-
-	/*
-	 * If we get killed while waiting for the HPD timeout, return
-	 * no modes found: we are not in a restartable path, so we
-	 * can't handle signals gracefully.
-	 */
-	if (tda998x_edid_delay_wait(priv))
-		return 0;
-
-	if (priv->rev == TDA19988)
-		reg_clear(priv, REG_TX4, TX4_PD_RAM);
-
-	edid = drm_do_get_edid(connector, read_edid_block, priv);
-
-	if (priv->rev == TDA19988)
-		reg_set(priv, REG_TX4, TX4_PD_RAM);
-
-	if (!edid) {
-		dev_warn(&priv->hdmi->dev, "failed to read EDID\n");
-		return 0;
-	}
-
-	drm_mode_connector_update_edid_property(connector, edid);
-	n = drm_add_edid_modes(connector, edid);
-	priv->is_hdmi_sink = drm_detect_hdmi_monitor(edid);
-	drm_edid_to_eld(connector, edid);
-
-	kfree(edid);
-
-	return n;
-}
-
 static void tda998x_encoder_set_polling(struct tda998x_priv *priv,
 					struct drm_connector *connector)
 {
@@ -1550,45 +1591,6 @@ static const struct drm_encoder_funcs tda998x_encoder_funcs = {
 	.destroy = tda998x_encoder_destroy,
 };
 
-static struct drm_encoder *
-tda998x_connector_best_encoder(struct drm_connector *connector)
-{
-	struct tda998x_priv *priv = conn_to_tda998x_priv(connector);
-
-	return &priv->encoder;
-}
-
-static
-const struct drm_connector_helper_funcs tda998x_connector_helper_funcs = {
-	.get_modes = tda998x_connector_get_modes,
-	.mode_valid = tda998x_connector_mode_valid,
-	.best_encoder = tda998x_connector_best_encoder,
-};
-
-static void tda998x_connector_destroy(struct drm_connector *connector)
-{
-	drm_connector_unregister(connector);
-	drm_connector_cleanup(connector);
-}
-
-static int tda998x_connector_dpms(struct drm_connector *connector, int mode)
-{
-	if (drm_core_check_feature(connector->dev, DRIVER_ATOMIC))
-		return drm_atomic_helper_connector_dpms(connector, mode);
-	else
-		return drm_helper_connector_dpms(connector, mode);
-}
-
-static const struct drm_connector_funcs tda998x_connector_funcs = {
-	.dpms = tda998x_connector_dpms,
-	.reset = drm_atomic_helper_connector_reset,
-	.fill_modes = drm_helper_probe_single_connector_modes,
-	.detect = tda998x_connector_detect,
-	.destroy = tda998x_connector_destroy,
-	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
-	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
-};
-
 static void tda998x_set_config(struct tda998x_priv *priv,
 			       const struct tda998x_encoder_params *p)
 {
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH RFC 07/10] drm/i2c: tda998x: separate connector initialisation
  2016-10-23 19:10 ` Russell King - ARM Linux
@ 2016-10-23 19:10   ` Russell King
  -1 siblings, 0 replies; 22+ messages in thread
From: Russell King @ 2016-10-23 19:10 UTC (permalink / raw)
  To: linux-arm-kernel

Separate out the connector initialisation from the rest of the drivers
initialisation.

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

diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
index bb5389fbd059..4379c6aa1c48 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -979,6 +979,37 @@ const struct drm_connector_helper_funcs tda998x_connector_helper_funcs = {
 	.best_encoder = tda998x_connector_best_encoder,
 };
 
+static int tda998x_connector_init(struct tda998x_priv *priv,
+				  struct drm_device *drm)
+{
+	struct drm_connector *connector = &priv->connector;
+	int ret;
+
+	connector->interlace_allowed = 1;
+
+	if (priv->hdmi->irq)
+		connector->polled = DRM_CONNECTOR_POLL_HPD;
+	else
+		connector->polled = DRM_CONNECTOR_POLL_CONNECT |
+			DRM_CONNECTOR_POLL_DISCONNECT;
+
+	drm_connector_helper_add(connector, &tda998x_connector_helper_funcs);
+	ret = drm_connector_init(drm, connector, &tda998x_connector_funcs,
+				 DRM_MODE_CONNECTOR_HDMIA);
+	if (ret)
+		return ret;
+
+	ret = drm_connector_register(&priv->connector);
+	if (ret) {
+		drm_connector_cleanup(&priv->connector);
+		return ret;
+	}
+
+	drm_mode_connector_attach_encoder(&priv->connector, &priv->encoder);
+
+	return 0;
+}
+
 /* DRM encoder functions */
 
 static void tda998x_encoder_dpms(struct drm_encoder *encoder, int mode)
@@ -1212,16 +1243,6 @@ tda998x_encoder_mode_set(struct drm_encoder *encoder,
 	mutex_unlock(&priv->audio_mutex);
 }
 
-static void tda998x_encoder_set_polling(struct tda998x_priv *priv,
-					struct drm_connector *connector)
-{
-	if (priv->hdmi->irq)
-		connector->polled = DRM_CONNECTOR_POLL_HPD;
-	else
-		connector->polled = DRM_CONNECTOR_POLL_CONNECT |
-			DRM_CONNECTOR_POLL_DISCONNECT;
-}
-
 static void tda998x_destroy(struct tda998x_priv *priv)
 {
 	/* disable all IRQs and free the IRQ handler */
@@ -1634,7 +1655,6 @@ static int tda998x_bind(struct device *dev, struct device *master, void *data)
 		crtcs = 1 << 0;
 	}
 
-	priv->connector.interlace_allowed = 1;
 	priv->encoder.possible_crtcs = crtcs;
 
 	ret = tda998x_create(client, priv);
@@ -1644,32 +1664,18 @@ static int tda998x_bind(struct device *dev, struct device *master, void *data)
 	if (!dev->of_node && params)
 		tda998x_set_config(priv, params);
 
-	tda998x_encoder_set_polling(priv, &priv->connector);
-
 	drm_encoder_helper_add(&priv->encoder, &tda998x_encoder_helper_funcs);
 	ret = drm_encoder_init(drm, &priv->encoder, &tda998x_encoder_funcs,
 			       DRM_MODE_ENCODER_TMDS, NULL);
 	if (ret)
 		goto err_encoder;
 
-	drm_connector_helper_add(&priv->connector,
-				 &tda998x_connector_helper_funcs);
-	ret = drm_connector_init(drm, &priv->connector,
-				 &tda998x_connector_funcs,
-				 DRM_MODE_CONNECTOR_HDMIA);
+	ret = tda998x_connector_init(priv, drm);
 	if (ret)
 		goto err_connector;
 
-	ret = drm_connector_register(&priv->connector);
-	if (ret)
-		goto err_sysfs;
-
-	drm_mode_connector_attach_encoder(&priv->connector, &priv->encoder);
-
 	return 0;
 
-err_sysfs:
-	drm_connector_cleanup(&priv->connector);
 err_connector:
 	drm_encoder_cleanup(&priv->encoder);
 err_encoder:
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH RFC 07/10] drm/i2c: tda998x: separate connector initialisation
@ 2016-10-23 19:10   ` Russell King
  0 siblings, 0 replies; 22+ messages in thread
From: Russell King @ 2016-10-23 19:10 UTC (permalink / raw)
  To: dri-devel, linux-arm-kernel

Separate out the connector initialisation from the rest of the drivers
initialisation.

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

diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
index bb5389fbd059..4379c6aa1c48 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -979,6 +979,37 @@ const struct drm_connector_helper_funcs tda998x_connector_helper_funcs = {
 	.best_encoder = tda998x_connector_best_encoder,
 };
 
+static int tda998x_connector_init(struct tda998x_priv *priv,
+				  struct drm_device *drm)
+{
+	struct drm_connector *connector = &priv->connector;
+	int ret;
+
+	connector->interlace_allowed = 1;
+
+	if (priv->hdmi->irq)
+		connector->polled = DRM_CONNECTOR_POLL_HPD;
+	else
+		connector->polled = DRM_CONNECTOR_POLL_CONNECT |
+			DRM_CONNECTOR_POLL_DISCONNECT;
+
+	drm_connector_helper_add(connector, &tda998x_connector_helper_funcs);
+	ret = drm_connector_init(drm, connector, &tda998x_connector_funcs,
+				 DRM_MODE_CONNECTOR_HDMIA);
+	if (ret)
+		return ret;
+
+	ret = drm_connector_register(&priv->connector);
+	if (ret) {
+		drm_connector_cleanup(&priv->connector);
+		return ret;
+	}
+
+	drm_mode_connector_attach_encoder(&priv->connector, &priv->encoder);
+
+	return 0;
+}
+
 /* DRM encoder functions */
 
 static void tda998x_encoder_dpms(struct drm_encoder *encoder, int mode)
@@ -1212,16 +1243,6 @@ tda998x_encoder_mode_set(struct drm_encoder *encoder,
 	mutex_unlock(&priv->audio_mutex);
 }
 
-static void tda998x_encoder_set_polling(struct tda998x_priv *priv,
-					struct drm_connector *connector)
-{
-	if (priv->hdmi->irq)
-		connector->polled = DRM_CONNECTOR_POLL_HPD;
-	else
-		connector->polled = DRM_CONNECTOR_POLL_CONNECT |
-			DRM_CONNECTOR_POLL_DISCONNECT;
-}
-
 static void tda998x_destroy(struct tda998x_priv *priv)
 {
 	/* disable all IRQs and free the IRQ handler */
@@ -1634,7 +1655,6 @@ static int tda998x_bind(struct device *dev, struct device *master, void *data)
 		crtcs = 1 << 0;
 	}
 
-	priv->connector.interlace_allowed = 1;
 	priv->encoder.possible_crtcs = crtcs;
 
 	ret = tda998x_create(client, priv);
@@ -1644,32 +1664,18 @@ static int tda998x_bind(struct device *dev, struct device *master, void *data)
 	if (!dev->of_node && params)
 		tda998x_set_config(priv, params);
 
-	tda998x_encoder_set_polling(priv, &priv->connector);
-
 	drm_encoder_helper_add(&priv->encoder, &tda998x_encoder_helper_funcs);
 	ret = drm_encoder_init(drm, &priv->encoder, &tda998x_encoder_funcs,
 			       DRM_MODE_ENCODER_TMDS, NULL);
 	if (ret)
 		goto err_encoder;
 
-	drm_connector_helper_add(&priv->connector,
-				 &tda998x_connector_helper_funcs);
-	ret = drm_connector_init(drm, &priv->connector,
-				 &tda998x_connector_funcs,
-				 DRM_MODE_CONNECTOR_HDMIA);
+	ret = tda998x_connector_init(priv, drm);
 	if (ret)
 		goto err_connector;
 
-	ret = drm_connector_register(&priv->connector);
-	if (ret)
-		goto err_sysfs;
-
-	drm_mode_connector_attach_encoder(&priv->connector, &priv->encoder);
-
 	return 0;
 
-err_sysfs:
-	drm_connector_cleanup(&priv->connector);
 err_connector:
 	drm_encoder_cleanup(&priv->encoder);
 err_encoder:
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH RFC 08/10] drm/i2c: tda998x: group audio functions together
  2016-10-23 19:10 ` Russell King - ARM Linux
@ 2016-10-23 19:11   ` Russell King
  -1 siblings, 0 replies; 22+ messages in thread
From: Russell King @ 2016-10-23 19:11 UTC (permalink / raw)
  To: linux-arm-kernel

Group the TDA998x audio functions together rather than split between
two different locations in the file, keeping like code together.

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

diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
index 4379c6aa1c48..6a7095b66a69 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -702,6 +702,8 @@ tda998x_write_avi(struct tda998x_priv *priv, struct drm_display_mode *mode)
 	tda998x_write_if(priv, DIP_IF_FLAGS_IF2, REG_IF2_HB0, &frame);
 }
 
+/* Audio support */
+
 static void tda998x_audio_mute(struct tda998x_priv *priv, bool on)
 {
 	if (on) {
@@ -820,6 +822,144 @@ tda998x_configure_audio(struct tda998x_priv *priv,
 	return tda998x_write_aif(priv, &params->cea);
 }
 
+static int tda998x_audio_hw_params(struct device *dev, void *data,
+				   struct hdmi_codec_daifmt *daifmt,
+				   struct hdmi_codec_params *params)
+{
+	struct tda998x_priv *priv = dev_get_drvdata(dev);
+	int i, ret;
+	struct tda998x_audio_params audio = {
+		.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)));
+
+	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;
+		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:
+		dev_err(dev, "%s: Invalid format %d\n", __func__, daifmt->fmt);
+		return -EINVAL;
+	}
+
+	if (audio.config == 0) {
+		dev_err(dev, "%s: No audio configutation found\n", __func__);
+		return -EINVAL;
+	}
+
+	mutex_lock(&priv->audio_mutex);
+	/* We must not program the TDA998x for audio if the sink is DVI. */
+	if (priv->is_hdmi_config)
+		ret = tda998x_configure_audio(priv, &audio);
+	else
+		ret = 0;
+
+	if (ret == 0)
+		priv->audio_params = audio;
+	mutex_unlock(&priv->audio_mutex);
+
+	return ret;
+}
+
+static void tda998x_audio_shutdown(struct device *dev, void *data)
+{
+	struct tda998x_priv *priv = dev_get_drvdata(dev);
+
+	mutex_lock(&priv->audio_mutex);
+
+	reg_write(priv, REG_ENA_AP, 0);
+
+	priv->audio_params.format = AFMT_UNUSED;
+
+	mutex_unlock(&priv->audio_mutex);
+}
+
+int tda998x_audio_digital_mute(struct device *dev, void *data, bool enable)
+{
+	struct tda998x_priv *priv = dev_get_drvdata(dev);
+
+	mutex_lock(&priv->audio_mutex);
+
+	tda998x_audio_mute(priv, enable);
+
+	mutex_unlock(&priv->audio_mutex);
+	return 0;
+}
+
+static int tda998x_audio_get_eld(struct device *dev, void *data,
+				 uint8_t *buf, size_t len)
+{
+	struct tda998x_priv *priv = dev_get_drvdata(dev);
+	struct drm_mode_config *config = &priv->encoder.dev->mode_config;
+	struct drm_connector *connector;
+	int ret = -ENODEV;
+
+	mutex_lock(&config->mutex);
+	list_for_each_entry(connector, &config->connector_list, head) {
+		if (&priv->encoder == connector->encoder) {
+			memcpy(buf, connector->eld,
+			       min(sizeof(connector->eld), len));
+			ret = 0;
+		}
+	}
+	mutex_unlock(&config->mutex);
+
+	return ret;
+}
+
+static const struct hdmi_codec_ops audio_codec_ops = {
+	.hw_params = tda998x_audio_hw_params,
+	.audio_shutdown = tda998x_audio_shutdown,
+	.digital_mute = tda998x_audio_digital_mute,
+	.get_eld = tda998x_audio_get_eld,
+};
+
+static int tda998x_audio_codec_init(struct tda998x_priv *priv,
+				    struct device *dev)
+{
+	struct hdmi_codec_pdata codec_data = {
+		.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;
+	}
+
+	priv->audio_pdev = platform_device_register_data(
+		dev, HDMI_CODEC_DRV_NAME, PLATFORM_DEVID_AUTO,
+		&codec_data, sizeof(codec_data));
+
+	return PTR_ERR_OR_ZERO(priv->audio_pdev);
+}
+
 /* DRM connector functions */
 
 static int tda998x_connector_dpms(struct drm_connector *connector, int mode)
@@ -1261,144 +1401,6 @@ static void tda998x_destroy(struct tda998x_priv *priv)
 	i2c_unregister_device(priv->cec);
 }
 
-static int tda998x_audio_hw_params(struct device *dev, void *data,
-				   struct hdmi_codec_daifmt *daifmt,
-				   struct hdmi_codec_params *params)
-{
-	struct tda998x_priv *priv = dev_get_drvdata(dev);
-	int i, ret;
-	struct tda998x_audio_params audio = {
-		.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)));
-
-	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;
-		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:
-		dev_err(dev, "%s: Invalid format %d\n", __func__, daifmt->fmt);
-		return -EINVAL;
-	}
-
-	if (audio.config == 0) {
-		dev_err(dev, "%s: No audio configutation found\n", __func__);
-		return -EINVAL;
-	}
-
-	mutex_lock(&priv->audio_mutex);
-	/* We must not program the TDA998x for audio if the sink is DVI. */
-	if (priv->is_hdmi_config)
-		ret = tda998x_configure_audio(priv, &audio);
-	else
-		ret = 0;
-
-	if (ret == 0)
-		priv->audio_params = audio;
-	mutex_unlock(&priv->audio_mutex);
-
-	return ret;
-}
-
-static void tda998x_audio_shutdown(struct device *dev, void *data)
-{
-	struct tda998x_priv *priv = dev_get_drvdata(dev);
-
-	mutex_lock(&priv->audio_mutex);
-
-	reg_write(priv, REG_ENA_AP, 0);
-
-	priv->audio_params.format = AFMT_UNUSED;
-
-	mutex_unlock(&priv->audio_mutex);
-}
-
-int tda998x_audio_digital_mute(struct device *dev, void *data, bool enable)
-{
-	struct tda998x_priv *priv = dev_get_drvdata(dev);
-
-	mutex_lock(&priv->audio_mutex);
-
-	tda998x_audio_mute(priv, enable);
-
-	mutex_unlock(&priv->audio_mutex);
-	return 0;
-}
-
-static int tda998x_audio_get_eld(struct device *dev, void *data,
-				 uint8_t *buf, size_t len)
-{
-	struct tda998x_priv *priv = dev_get_drvdata(dev);
-	struct drm_mode_config *config = &priv->encoder.dev->mode_config;
-	struct drm_connector *connector;
-	int ret = -ENODEV;
-
-	mutex_lock(&config->mutex);
-	list_for_each_entry(connector, &config->connector_list, head) {
-		if (&priv->encoder == connector->encoder) {
-			memcpy(buf, connector->eld,
-			       min(sizeof(connector->eld), len));
-			ret = 0;
-		}
-	}
-	mutex_unlock(&config->mutex);
-
-	return ret;
-}
-
-static const struct hdmi_codec_ops audio_codec_ops = {
-	.hw_params = tda998x_audio_hw_params,
-	.audio_shutdown = tda998x_audio_shutdown,
-	.digital_mute = tda998x_audio_digital_mute,
-	.get_eld = tda998x_audio_get_eld,
-};
-
-static int tda998x_audio_codec_init(struct tda998x_priv *priv,
-				    struct device *dev)
-{
-	struct hdmi_codec_pdata codec_data = {
-		.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;
-	}
-
-	priv->audio_pdev = platform_device_register_data(
-		dev, HDMI_CODEC_DRV_NAME, PLATFORM_DEVID_AUTO,
-		&codec_data, sizeof(codec_data));
-
-	return PTR_ERR_OR_ZERO(priv->audio_pdev);
-}
-
 /* I2C driver functions */
 
 static int tda998x_get_audio_ports(struct tda998x_priv *priv,
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH RFC 08/10] drm/i2c: tda998x: group audio functions together
@ 2016-10-23 19:11   ` Russell King
  0 siblings, 0 replies; 22+ messages in thread
From: Russell King @ 2016-10-23 19:11 UTC (permalink / raw)
  To: dri-devel, linux-arm-kernel

Group the TDA998x audio functions together rather than split between
two different locations in the file, keeping like code together.

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

diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
index 4379c6aa1c48..6a7095b66a69 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -702,6 +702,8 @@ tda998x_write_avi(struct tda998x_priv *priv, struct drm_display_mode *mode)
 	tda998x_write_if(priv, DIP_IF_FLAGS_IF2, REG_IF2_HB0, &frame);
 }
 
+/* Audio support */
+
 static void tda998x_audio_mute(struct tda998x_priv *priv, bool on)
 {
 	if (on) {
@@ -820,6 +822,144 @@ tda998x_configure_audio(struct tda998x_priv *priv,
 	return tda998x_write_aif(priv, &params->cea);
 }
 
+static int tda998x_audio_hw_params(struct device *dev, void *data,
+				   struct hdmi_codec_daifmt *daifmt,
+				   struct hdmi_codec_params *params)
+{
+	struct tda998x_priv *priv = dev_get_drvdata(dev);
+	int i, ret;
+	struct tda998x_audio_params audio = {
+		.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)));
+
+	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;
+		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:
+		dev_err(dev, "%s: Invalid format %d\n", __func__, daifmt->fmt);
+		return -EINVAL;
+	}
+
+	if (audio.config == 0) {
+		dev_err(dev, "%s: No audio configutation found\n", __func__);
+		return -EINVAL;
+	}
+
+	mutex_lock(&priv->audio_mutex);
+	/* We must not program the TDA998x for audio if the sink is DVI. */
+	if (priv->is_hdmi_config)
+		ret = tda998x_configure_audio(priv, &audio);
+	else
+		ret = 0;
+
+	if (ret == 0)
+		priv->audio_params = audio;
+	mutex_unlock(&priv->audio_mutex);
+
+	return ret;
+}
+
+static void tda998x_audio_shutdown(struct device *dev, void *data)
+{
+	struct tda998x_priv *priv = dev_get_drvdata(dev);
+
+	mutex_lock(&priv->audio_mutex);
+
+	reg_write(priv, REG_ENA_AP, 0);
+
+	priv->audio_params.format = AFMT_UNUSED;
+
+	mutex_unlock(&priv->audio_mutex);
+}
+
+int tda998x_audio_digital_mute(struct device *dev, void *data, bool enable)
+{
+	struct tda998x_priv *priv = dev_get_drvdata(dev);
+
+	mutex_lock(&priv->audio_mutex);
+
+	tda998x_audio_mute(priv, enable);
+
+	mutex_unlock(&priv->audio_mutex);
+	return 0;
+}
+
+static int tda998x_audio_get_eld(struct device *dev, void *data,
+				 uint8_t *buf, size_t len)
+{
+	struct tda998x_priv *priv = dev_get_drvdata(dev);
+	struct drm_mode_config *config = &priv->encoder.dev->mode_config;
+	struct drm_connector *connector;
+	int ret = -ENODEV;
+
+	mutex_lock(&config->mutex);
+	list_for_each_entry(connector, &config->connector_list, head) {
+		if (&priv->encoder == connector->encoder) {
+			memcpy(buf, connector->eld,
+			       min(sizeof(connector->eld), len));
+			ret = 0;
+		}
+	}
+	mutex_unlock(&config->mutex);
+
+	return ret;
+}
+
+static const struct hdmi_codec_ops audio_codec_ops = {
+	.hw_params = tda998x_audio_hw_params,
+	.audio_shutdown = tda998x_audio_shutdown,
+	.digital_mute = tda998x_audio_digital_mute,
+	.get_eld = tda998x_audio_get_eld,
+};
+
+static int tda998x_audio_codec_init(struct tda998x_priv *priv,
+				    struct device *dev)
+{
+	struct hdmi_codec_pdata codec_data = {
+		.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;
+	}
+
+	priv->audio_pdev = platform_device_register_data(
+		dev, HDMI_CODEC_DRV_NAME, PLATFORM_DEVID_AUTO,
+		&codec_data, sizeof(codec_data));
+
+	return PTR_ERR_OR_ZERO(priv->audio_pdev);
+}
+
 /* DRM connector functions */
 
 static int tda998x_connector_dpms(struct drm_connector *connector, int mode)
@@ -1261,144 +1401,6 @@ static void tda998x_destroy(struct tda998x_priv *priv)
 	i2c_unregister_device(priv->cec);
 }
 
-static int tda998x_audio_hw_params(struct device *dev, void *data,
-				   struct hdmi_codec_daifmt *daifmt,
-				   struct hdmi_codec_params *params)
-{
-	struct tda998x_priv *priv = dev_get_drvdata(dev);
-	int i, ret;
-	struct tda998x_audio_params audio = {
-		.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)));
-
-	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;
-		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:
-		dev_err(dev, "%s: Invalid format %d\n", __func__, daifmt->fmt);
-		return -EINVAL;
-	}
-
-	if (audio.config == 0) {
-		dev_err(dev, "%s: No audio configutation found\n", __func__);
-		return -EINVAL;
-	}
-
-	mutex_lock(&priv->audio_mutex);
-	/* We must not program the TDA998x for audio if the sink is DVI. */
-	if (priv->is_hdmi_config)
-		ret = tda998x_configure_audio(priv, &audio);
-	else
-		ret = 0;
-
-	if (ret == 0)
-		priv->audio_params = audio;
-	mutex_unlock(&priv->audio_mutex);
-
-	return ret;
-}
-
-static void tda998x_audio_shutdown(struct device *dev, void *data)
-{
-	struct tda998x_priv *priv = dev_get_drvdata(dev);
-
-	mutex_lock(&priv->audio_mutex);
-
-	reg_write(priv, REG_ENA_AP, 0);
-
-	priv->audio_params.format = AFMT_UNUSED;
-
-	mutex_unlock(&priv->audio_mutex);
-}
-
-int tda998x_audio_digital_mute(struct device *dev, void *data, bool enable)
-{
-	struct tda998x_priv *priv = dev_get_drvdata(dev);
-
-	mutex_lock(&priv->audio_mutex);
-
-	tda998x_audio_mute(priv, enable);
-
-	mutex_unlock(&priv->audio_mutex);
-	return 0;
-}
-
-static int tda998x_audio_get_eld(struct device *dev, void *data,
-				 uint8_t *buf, size_t len)
-{
-	struct tda998x_priv *priv = dev_get_drvdata(dev);
-	struct drm_mode_config *config = &priv->encoder.dev->mode_config;
-	struct drm_connector *connector;
-	int ret = -ENODEV;
-
-	mutex_lock(&config->mutex);
-	list_for_each_entry(connector, &config->connector_list, head) {
-		if (&priv->encoder == connector->encoder) {
-			memcpy(buf, connector->eld,
-			       min(sizeof(connector->eld), len));
-			ret = 0;
-		}
-	}
-	mutex_unlock(&config->mutex);
-
-	return ret;
-}
-
-static const struct hdmi_codec_ops audio_codec_ops = {
-	.hw_params = tda998x_audio_hw_params,
-	.audio_shutdown = tda998x_audio_shutdown,
-	.digital_mute = tda998x_audio_digital_mute,
-	.get_eld = tda998x_audio_get_eld,
-};
-
-static int tda998x_audio_codec_init(struct tda998x_priv *priv,
-				    struct device *dev)
-{
-	struct hdmi_codec_pdata codec_data = {
-		.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;
-	}
-
-	priv->audio_pdev = platform_device_register_data(
-		dev, HDMI_CODEC_DRV_NAME, PLATFORM_DEVID_AUTO,
-		&codec_data, sizeof(codec_data));
-
-	return PTR_ERR_OR_ZERO(priv->audio_pdev);
-}
-
 /* I2C driver functions */
 
 static int tda998x_get_audio_ports(struct tda998x_priv *priv,
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH RFC 09/10] drm/i2c: tda998x: remove complexity from tda998x_audio_get_eld()
  2016-10-23 19:10 ` Russell King - ARM Linux
@ 2016-10-23 19:11   ` Russell King
  -1 siblings, 0 replies; 22+ messages in thread
From: Russell King @ 2016-10-23 19:11 UTC (permalink / raw)
  To: linux-arm-kernel

tda998x_audio_get_eld() is needlessly complex - the connector associated
with the encoder is always our own priv->connector.  Remove this
complexity, but ensure that there are no races when copying out the ELD.

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

diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
index 6a7095b66a69..547cf99ac32d 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -911,21 +911,13 @@ static int tda998x_audio_get_eld(struct device *dev, void *data,
 				 uint8_t *buf, size_t len)
 {
 	struct tda998x_priv *priv = dev_get_drvdata(dev);
-	struct drm_mode_config *config = &priv->encoder.dev->mode_config;
-	struct drm_connector *connector;
-	int ret = -ENODEV;
-
-	mutex_lock(&config->mutex);
-	list_for_each_entry(connector, &config->connector_list, head) {
-		if (&priv->encoder == connector->encoder) {
-			memcpy(buf, connector->eld,
-			       min(sizeof(connector->eld), len));
-			ret = 0;
-		}
-	}
-	mutex_unlock(&config->mutex);
 
-	return ret;
+	mutex_lock(&priv->audio_mutex);
+	memcpy(buf, priv->connector.eld,
+	       min(sizeof(priv->connector.eld), len));
+	mutex_unlock(&priv->audio_mutex);
+
+	return 0;
 }
 
 static const struct hdmi_codec_ops audio_codec_ops = {
@@ -1082,7 +1074,10 @@ static int tda998x_connector_get_modes(struct drm_connector *connector)
 	drm_mode_connector_update_edid_property(connector, edid);
 	n = drm_add_edid_modes(connector, edid);
 	priv->is_hdmi_sink = drm_detect_hdmi_monitor(edid);
+
+	mutex_lock(&priv->audio_mutex);
 	drm_edid_to_eld(connector, edid);
+	mutex_unlock(&priv->audio_mutex);
 
 	kfree(edid);
 
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH RFC 09/10] drm/i2c: tda998x: remove complexity from tda998x_audio_get_eld()
@ 2016-10-23 19:11   ` Russell King
  0 siblings, 0 replies; 22+ messages in thread
From: Russell King @ 2016-10-23 19:11 UTC (permalink / raw)
  To: dri-devel, linux-arm-kernel

tda998x_audio_get_eld() is needlessly complex - the connector associated
with the encoder is always our own priv->connector.  Remove this
complexity, but ensure that there are no races when copying out the ELD.

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

diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
index 6a7095b66a69..547cf99ac32d 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -911,21 +911,13 @@ static int tda998x_audio_get_eld(struct device *dev, void *data,
 				 uint8_t *buf, size_t len)
 {
 	struct tda998x_priv *priv = dev_get_drvdata(dev);
-	struct drm_mode_config *config = &priv->encoder.dev->mode_config;
-	struct drm_connector *connector;
-	int ret = -ENODEV;
-
-	mutex_lock(&config->mutex);
-	list_for_each_entry(connector, &config->connector_list, head) {
-		if (&priv->encoder == connector->encoder) {
-			memcpy(buf, connector->eld,
-			       min(sizeof(connector->eld), len));
-			ret = 0;
-		}
-	}
-	mutex_unlock(&config->mutex);
 
-	return ret;
+	mutex_lock(&priv->audio_mutex);
+	memcpy(buf, priv->connector.eld,
+	       min(sizeof(priv->connector.eld), len));
+	mutex_unlock(&priv->audio_mutex);
+
+	return 0;
 }
 
 static const struct hdmi_codec_ops audio_codec_ops = {
@@ -1082,7 +1074,10 @@ static int tda998x_connector_get_modes(struct drm_connector *connector)
 	drm_mode_connector_update_edid_property(connector, edid);
 	n = drm_add_edid_modes(connector, edid);
 	priv->is_hdmi_sink = drm_detect_hdmi_monitor(edid);
+
+	mutex_lock(&priv->audio_mutex);
 	drm_edid_to_eld(connector, edid);
+	mutex_unlock(&priv->audio_mutex);
 
 	kfree(edid);
 
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH RFC 10/10] drm/i2c: tda998x: switch to boolean is_on
  2016-10-23 19:10 ` Russell King - ARM Linux
@ 2016-10-23 19:11   ` Russell King
  -1 siblings, 0 replies; 22+ messages in thread
From: Russell King @ 2016-10-23 19:11 UTC (permalink / raw)
  To: linux-arm-kernel

Rather than storing the DPMS mode (which will always be on or off) use a
boolean to store this instead.

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

diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
index 547cf99ac32d..1f9a25fe17f3 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -42,7 +42,7 @@ struct tda998x_priv {
 	struct mutex mutex;
 	u16 rev;
 	u8 current_page;
-	int dpms;
+	bool is_on;
 	bool is_hdmi_sink;
 	bool is_hdmi_config;
 	u8 vip_cntrl_0;
@@ -1150,16 +1150,15 @@ static int tda998x_connector_init(struct tda998x_priv *priv,
 static void tda998x_encoder_dpms(struct drm_encoder *encoder, int mode)
 {
 	struct tda998x_priv *priv = enc_to_tda998x_priv(encoder);
+	bool on;
 
 	/* we only care about on or off: */
-	if (mode != DRM_MODE_DPMS_ON)
-		mode = DRM_MODE_DPMS_OFF;
+	on = mode == DRM_MODE_DPMS_ON;
 
-	if (mode == priv->dpms)
+	if (on == priv->is_on)
 		return;
 
-	switch (mode) {
-	case DRM_MODE_DPMS_ON:
+	if (on) {
 		/* enable video ports, audio will be enabled later */
 		reg_write(priv, REG_ENA_VP_0, 0xff);
 		reg_write(priv, REG_ENA_VP_1, 0xff);
@@ -1168,16 +1167,16 @@ static void tda998x_encoder_dpms(struct drm_encoder *encoder, int mode)
 		reg_write(priv, REG_VIP_CNTRL_0, priv->vip_cntrl_0);
 		reg_write(priv, REG_VIP_CNTRL_1, priv->vip_cntrl_1);
 		reg_write(priv, REG_VIP_CNTRL_2, priv->vip_cntrl_2);
-		break;
-	case DRM_MODE_DPMS_OFF:
+
+		priv->is_on = true;
+	} else {
 		/* disable video ports */
 		reg_write(priv, REG_ENA_VP_0, 0x00);
 		reg_write(priv, REG_ENA_VP_1, 0x00);
 		reg_write(priv, REG_ENA_VP_2, 0x00);
-		break;
-	}
 
-	priv->dpms = mode;
+		priv->is_on = false;
+	}
 }
 
 static void
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH RFC 10/10] drm/i2c: tda998x: switch to boolean is_on
@ 2016-10-23 19:11   ` Russell King
  0 siblings, 0 replies; 22+ messages in thread
From: Russell King @ 2016-10-23 19:11 UTC (permalink / raw)
  To: dri-devel, linux-arm-kernel

Rather than storing the DPMS mode (which will always be on or off) use a
boolean to store this instead.

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

diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
index 547cf99ac32d..1f9a25fe17f3 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -42,7 +42,7 @@ struct tda998x_priv {
 	struct mutex mutex;
 	u16 rev;
 	u8 current_page;
-	int dpms;
+	bool is_on;
 	bool is_hdmi_sink;
 	bool is_hdmi_config;
 	u8 vip_cntrl_0;
@@ -1150,16 +1150,15 @@ static int tda998x_connector_init(struct tda998x_priv *priv,
 static void tda998x_encoder_dpms(struct drm_encoder *encoder, int mode)
 {
 	struct tda998x_priv *priv = enc_to_tda998x_priv(encoder);
+	bool on;
 
 	/* we only care about on or off: */
-	if (mode != DRM_MODE_DPMS_ON)
-		mode = DRM_MODE_DPMS_OFF;
+	on = mode == DRM_MODE_DPMS_ON;
 
-	if (mode == priv->dpms)
+	if (on == priv->is_on)
 		return;
 
-	switch (mode) {
-	case DRM_MODE_DPMS_ON:
+	if (on) {
 		/* enable video ports, audio will be enabled later */
 		reg_write(priv, REG_ENA_VP_0, 0xff);
 		reg_write(priv, REG_ENA_VP_1, 0xff);
@@ -1168,16 +1167,16 @@ static void tda998x_encoder_dpms(struct drm_encoder *encoder, int mode)
 		reg_write(priv, REG_VIP_CNTRL_0, priv->vip_cntrl_0);
 		reg_write(priv, REG_VIP_CNTRL_1, priv->vip_cntrl_1);
 		reg_write(priv, REG_VIP_CNTRL_2, priv->vip_cntrl_2);
-		break;
-	case DRM_MODE_DPMS_OFF:
+
+		priv->is_on = true;
+	} else {
 		/* disable video ports */
 		reg_write(priv, REG_ENA_VP_0, 0x00);
 		reg_write(priv, REG_ENA_VP_1, 0x00);
 		reg_write(priv, REG_ENA_VP_2, 0x00);
-		break;
-	}
 
-	priv->dpms = mode;
+		priv->is_on = false;
+	}
 }
 
 static void
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2016-10-23 19:11 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-10-23 19:10 [PATCH RFC 0/10] tda998x initial cleanups from bridge conversion Russell King - ARM Linux
2016-10-23 19:10 ` Russell King - ARM Linux
2016-10-23 19:10 ` [PATCH RFC 01/10] drm/i2c: tda998x: avoid race in tda998x_encoder_mode_set() Russell King
2016-10-23 19:10   ` Russell King
2016-10-23 19:10 ` [PATCH RFC 02/10] drm/i2c: tda998x: avoid configuring audio for DVI mode Russell King
2016-10-23 19:10   ` Russell King
2016-10-23 19:10 ` [PATCH RFC 03/10] drm/i2c: tda998x: avoid racy access to mode clock Russell King
2016-10-23 19:10   ` Russell King
2016-10-23 19:10 ` [PATCH RFC 04/10] drm/i2c: tda998x: correct function name in comments Russell King
2016-10-23 19:10   ` Russell King
2016-10-23 19:10 ` [PATCH RFC 05/10] drm/i2c: tda998x: move and rename tda998x_encoder_set_config() Russell King
2016-10-23 19:10   ` Russell King
2016-10-23 19:10 ` [PATCH RFC 06/10] drm/i2c: tda998x: group connector functions and funcs together Russell King
2016-10-23 19:10   ` Russell King
2016-10-23 19:10 ` [PATCH RFC 07/10] drm/i2c: tda998x: separate connector initialisation Russell King
2016-10-23 19:10   ` Russell King
2016-10-23 19:11 ` [PATCH RFC 08/10] drm/i2c: tda998x: group audio functions together Russell King
2016-10-23 19:11   ` Russell King
2016-10-23 19:11 ` [PATCH RFC 09/10] drm/i2c: tda998x: remove complexity from tda998x_audio_get_eld() Russell King
2016-10-23 19:11   ` Russell King
2016-10-23 19:11 ` [PATCH RFC 10/10] drm/i2c: tda998x: switch to boolean is_on Russell King
2016-10-23 19:11   ` Russell King

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.