All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/4 v2] Audio support for adv7511 hdmi bridge
@ 2016-08-29 23:41 ` John Stultz
  0 siblings, 0 replies; 26+ messages in thread
From: John Stultz @ 2016-08-29 23:41 UTC (permalink / raw)
  To: lkml
  Cc: John Stultz, David Airlie, Archit Taneja, Laurent Pinchart,
	Wolfram Sang, Srinivas Kandagatla, Ville Syrjälä,
	Boris Brezillon, Andy Green, Dave Long, Guodong Xu, Zhangfei Gao,
	Mark Brown, Lars-Peter Clausen, Jose Abreu, dri-devel

This is another swing at getting the adv7511 hdmi bridge
audio support reviewed.

I've taken the core audio work done by Lars-Peter Clausen, and
adapted by Srinivas Kandagatla and Archit Taneja, and tried to
rework it to use the hdmi-codec sound driver.

This patchset, along with the i2s driver and dts changes allows
HDMI audio to work on the HiKey board.

I'd really appreciate any thoughts or feedback.

New in v2:
* Integrated Srinivas' review feedback

thanks
-john

Cc: David Airlie <airlied@linux.ie>
Cc: Archit Taneja <architt@codeaurora.org>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
Cc: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
Cc: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
Cc: Boris Brezillon <boris.brezillon@free-electrons.com>
Cc: Andy Green <andy@warmcat.com>
Cc: Dave Long <dave.long@linaro.org>
Cc: Guodong Xu <guodong.xu@linaro.org>
Cc: Zhangfei Gao <zhangfei.gao@linaro.org>
Cc: Mark Brown <broonie@kernel.org>
Cc: Lars-Peter Clausen <lars@metafoo.de>
Cc: Jose Abreu <joabreu@synopsys.com>
Cc: dri-devel@lists.freedesktop.org

Andy Green (1):
  drm/bridge: adv7511: Initialize audio packet on adv7533

Archit Taneja (1):
  drm/bridge: adv7511: Move the common data structures to header file

John Stultz (1):
  drm/bridge: adv7511: Add Audio support.

Srinivas Kandagatla (1):
  drm/bridge: adv7511: Enable the audio data and clock pads on adv7533

 drivers/gpu/drm/bridge/adv7511/Kconfig         |   1 +
 drivers/gpu/drm/bridge/adv7511/Makefile        |   2 +-
 drivers/gpu/drm/bridge/adv7511/adv7511.h       |  13 ++
 drivers/gpu/drm/bridge/adv7511/adv7511_audio.c | 199 +++++++++++++++++++++++++
 drivers/gpu/drm/bridge/adv7511/adv7511_drv.c   |   9 +-
 drivers/gpu/drm/bridge/adv7511/adv7533.c       |  23 +++
 6 files changed, 244 insertions(+), 3 deletions(-)
 create mode 100644 drivers/gpu/drm/bridge/adv7511/adv7511_audio.c

-- 
1.9.1

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

* [PATCH 0/4 v2] Audio support for adv7511 hdmi bridge
@ 2016-08-29 23:41 ` John Stultz
  0 siblings, 0 replies; 26+ messages in thread
From: John Stultz @ 2016-08-29 23:41 UTC (permalink / raw)
  To: lkml
  Cc: Guodong Xu, dri-devel, Jose Abreu, Wolfram Sang, Mark Brown,
	Laurent Pinchart, Zhangfei Gao, Andy Green, Srinivas Kandagatla,
	Dave Long

This is another swing at getting the adv7511 hdmi bridge
audio support reviewed.

I've taken the core audio work done by Lars-Peter Clausen, and
adapted by Srinivas Kandagatla and Archit Taneja, and tried to
rework it to use the hdmi-codec sound driver.

This patchset, along with the i2s driver and dts changes allows
HDMI audio to work on the HiKey board.

I'd really appreciate any thoughts or feedback.

New in v2:
* Integrated Srinivas' review feedback

thanks
-john

Cc: David Airlie <airlied@linux.ie>
Cc: Archit Taneja <architt@codeaurora.org>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
Cc: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
Cc: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
Cc: Boris Brezillon <boris.brezillon@free-electrons.com>
Cc: Andy Green <andy@warmcat.com>
Cc: Dave Long <dave.long@linaro.org>
Cc: Guodong Xu <guodong.xu@linaro.org>
Cc: Zhangfei Gao <zhangfei.gao@linaro.org>
Cc: Mark Brown <broonie@kernel.org>
Cc: Lars-Peter Clausen <lars@metafoo.de>
Cc: Jose Abreu <joabreu@synopsys.com>
Cc: dri-devel@lists.freedesktop.org

Andy Green (1):
  drm/bridge: adv7511: Initialize audio packet on adv7533

Archit Taneja (1):
  drm/bridge: adv7511: Move the common data structures to header file

John Stultz (1):
  drm/bridge: adv7511: Add Audio support.

Srinivas Kandagatla (1):
  drm/bridge: adv7511: Enable the audio data and clock pads on adv7533

 drivers/gpu/drm/bridge/adv7511/Kconfig         |   1 +
 drivers/gpu/drm/bridge/adv7511/Makefile        |   2 +-
 drivers/gpu/drm/bridge/adv7511/adv7511.h       |  13 ++
 drivers/gpu/drm/bridge/adv7511/adv7511_audio.c | 199 +++++++++++++++++++++++++
 drivers/gpu/drm/bridge/adv7511/adv7511_drv.c   |   9 +-
 drivers/gpu/drm/bridge/adv7511/adv7533.c       |  23 +++
 6 files changed, 244 insertions(+), 3 deletions(-)
 create mode 100644 drivers/gpu/drm/bridge/adv7511/adv7511_audio.c

-- 
1.9.1

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

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

* [PATCH 1/4 v2] drm/bridge: adv7511: Move the common data structures to header file
  2016-08-29 23:41 ` John Stultz
@ 2016-08-29 23:41   ` John Stultz
  -1 siblings, 0 replies; 26+ messages in thread
From: John Stultz @ 2016-08-29 23:41 UTC (permalink / raw)
  To: lkml
  Cc: Archit Taneja, David Airlie, Laurent Pinchart, Wolfram Sang,
	Srinivas Kandagatla, Ville Syrjälä,
	Boris Brezillon, Andy Green, Dave Long, Guodong Xu, Zhangfei Gao,
	Mark Brown, Lars-Peter Clausen, Jose Abreu, dri-devel,
	John Stultz

From: Archit Taneja <architt@codeaurora.org>

This patch moves the adv7511 data structure to header file so that the
audio driver file could use it.

Cc: David Airlie <airlied@linux.ie>
Cc: Archit Taneja <architt@codeaurora.org>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
Cc: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
Cc: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
Cc: Boris Brezillon <boris.brezillon@free-electrons.com>
Cc: Andy Green <andy@warmcat.com>
Cc: Dave Long <dave.long@linaro.org>
Cc: Guodong Xu <guodong.xu@linaro.org>
Cc: Zhangfei Gao <zhangfei.gao@linaro.org>
Cc: Mark Brown <broonie@kernel.org>
Cc: Lars-Peter Clausen <lars@metafoo.de>
Cc: Jose Abreu <joabreu@synopsys.com>
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 drivers/gpu/drm/bridge/adv7511/adv7511.h     | 8 ++++++++
 drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 4 ++--
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511.h b/drivers/gpu/drm/bridge/adv7511/adv7511.h
index 161c923..c7002a0 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511.h
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511.h
@@ -16,6 +16,14 @@
 #include <drm/drm_crtc_helper.h>
 #include <drm/drm_mipi_dsi.h>
 
+#include <drm/drm_crtc_helper.h>
+
+struct regmap;
+struct adv7511;
+
+int adv7511_packet_enable(struct adv7511 *adv7511, unsigned int packet);
+int adv7511_packet_disable(struct adv7511 *adv7511, unsigned int packet);
+
 #define ADV7511_REG_CHIP_REVISION		0x00
 #define ADV7511_REG_N0				0x01
 #define ADV7511_REG_N1				0x02
diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
index ec8fb2e..f8eb7f8 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
@@ -160,7 +160,7 @@ static void adv7511_set_colormap(struct adv7511 *adv7511, bool enable,
 			   ADV7511_CSC_UPDATE_MODE, 0);
 }
 
-static int adv7511_packet_enable(struct adv7511 *adv7511, unsigned int packet)
+int adv7511_packet_enable(struct adv7511 *adv7511, unsigned int packet)
 {
 	if (packet & 0xff)
 		regmap_update_bits(adv7511->regmap, ADV7511_REG_PACKET_ENABLE0,
@@ -175,7 +175,7 @@ static int adv7511_packet_enable(struct adv7511 *adv7511, unsigned int packet)
 	return 0;
 }
 
-static int adv7511_packet_disable(struct adv7511 *adv7511, unsigned int packet)
+int adv7511_packet_disable(struct adv7511 *adv7511, unsigned int packet)
 {
 	if (packet & 0xff)
 		regmap_update_bits(adv7511->regmap, ADV7511_REG_PACKET_ENABLE0,
-- 
1.9.1

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

* [PATCH 1/4 v2] drm/bridge: adv7511: Move the common data structures to header file
@ 2016-08-29 23:41   ` John Stultz
  0 siblings, 0 replies; 26+ messages in thread
From: John Stultz @ 2016-08-29 23:41 UTC (permalink / raw)
  To: lkml
  Cc: Guodong Xu, dri-devel, Jose Abreu, Wolfram Sang, Mark Brown,
	Srinivas Kandagatla, Laurent Pinchart, Andy Green, Zhangfei Gao,
	Dave Long

From: Archit Taneja <architt@codeaurora.org>

This patch moves the adv7511 data structure to header file so that the
audio driver file could use it.

Cc: David Airlie <airlied@linux.ie>
Cc: Archit Taneja <architt@codeaurora.org>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
Cc: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
Cc: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
Cc: Boris Brezillon <boris.brezillon@free-electrons.com>
Cc: Andy Green <andy@warmcat.com>
Cc: Dave Long <dave.long@linaro.org>
Cc: Guodong Xu <guodong.xu@linaro.org>
Cc: Zhangfei Gao <zhangfei.gao@linaro.org>
Cc: Mark Brown <broonie@kernel.org>
Cc: Lars-Peter Clausen <lars@metafoo.de>
Cc: Jose Abreu <joabreu@synopsys.com>
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 drivers/gpu/drm/bridge/adv7511/adv7511.h     | 8 ++++++++
 drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 4 ++--
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511.h b/drivers/gpu/drm/bridge/adv7511/adv7511.h
index 161c923..c7002a0 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511.h
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511.h
@@ -16,6 +16,14 @@
 #include <drm/drm_crtc_helper.h>
 #include <drm/drm_mipi_dsi.h>
 
+#include <drm/drm_crtc_helper.h>
+
+struct regmap;
+struct adv7511;
+
+int adv7511_packet_enable(struct adv7511 *adv7511, unsigned int packet);
+int adv7511_packet_disable(struct adv7511 *adv7511, unsigned int packet);
+
 #define ADV7511_REG_CHIP_REVISION		0x00
 #define ADV7511_REG_N0				0x01
 #define ADV7511_REG_N1				0x02
diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
index ec8fb2e..f8eb7f8 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
@@ -160,7 +160,7 @@ static void adv7511_set_colormap(struct adv7511 *adv7511, bool enable,
 			   ADV7511_CSC_UPDATE_MODE, 0);
 }
 
-static int adv7511_packet_enable(struct adv7511 *adv7511, unsigned int packet)
+int adv7511_packet_enable(struct adv7511 *adv7511, unsigned int packet)
 {
 	if (packet & 0xff)
 		regmap_update_bits(adv7511->regmap, ADV7511_REG_PACKET_ENABLE0,
@@ -175,7 +175,7 @@ static int adv7511_packet_enable(struct adv7511 *adv7511, unsigned int packet)
 	return 0;
 }
 
-static int adv7511_packet_disable(struct adv7511 *adv7511, unsigned int packet)
+int adv7511_packet_disable(struct adv7511 *adv7511, unsigned int packet)
 {
 	if (packet & 0xff)
 		regmap_update_bits(adv7511->regmap, ADV7511_REG_PACKET_ENABLE0,
-- 
1.9.1

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

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

* [PATCH 2/4 v2] drm/bridge: adv7511: Add Audio support.
  2016-08-29 23:41 ` John Stultz
  (?)
  (?)
@ 2016-08-29 23:41 ` John Stultz
  2016-08-30  9:21     ` Laurent Pinchart
  -1 siblings, 1 reply; 26+ messages in thread
From: John Stultz @ 2016-08-29 23:41 UTC (permalink / raw)
  To: lkml
  Cc: John Stultz, David Airlie, Archit Taneja, Laurent Pinchart,
	Wolfram Sang, Srinivas Kandagatla, Ville Syrjälä,
	Boris Brezillon, Andy Green, Dave Long, Guodong Xu, Zhangfei Gao,
	Mark Brown, Lars-Peter Clausen, Jose Abreu, dri-devel

This patch adds support to Audio for both adv7511 and adv7533
bridge chips.

This patch was originally from [1] by Lars-Peter Clausen <lars@metafoo.de>
and was adapted by Archit Taneja <architt@codeaurora.org> and
Srinivas Kandagatla <srinivas.kandagatla@linaro.org>.

Then I heavily reworked it to use the hdmi-codec driver. So credit
to them, but blame to me.

[1] https://github.com/analogdevicesinc/linux/blob/xcomm_zynq/drivers/gpu/drm/i2c/adv7511_audio.c

Cc: David Airlie <airlied@linux.ie>
Cc: Archit Taneja <architt@codeaurora.org>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
Cc: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
Cc: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
Cc: Boris Brezillon <boris.brezillon@free-electrons.com>
Cc: Andy Green <andy@warmcat.com>
Cc: Dave Long <dave.long@linaro.org>
Cc: Guodong Xu <guodong.xu@linaro.org>
Cc: Zhangfei Gao <zhangfei.gao@linaro.org>
Cc: Mark Brown <broonie@kernel.org>
Cc: Lars-Peter Clausen <lars@metafoo.de>
Cc: Jose Abreu <joabreu@synopsys.com>
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
v2: Integrated feedback from Srinivas

 drivers/gpu/drm/bridge/adv7511/Kconfig         |   1 +
 drivers/gpu/drm/bridge/adv7511/Makefile        |   2 +-
 drivers/gpu/drm/bridge/adv7511/adv7511.h       |   5 +
 drivers/gpu/drm/bridge/adv7511/adv7511_audio.c | 199 +++++++++++++++++++++++++
 drivers/gpu/drm/bridge/adv7511/adv7511_drv.c   |   5 +
 5 files changed, 211 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/bridge/adv7511/adv7511_audio.c

diff --git a/drivers/gpu/drm/bridge/adv7511/Kconfig b/drivers/gpu/drm/bridge/adv7511/Kconfig
index d2b0499..e3975e9 100644
--- a/drivers/gpu/drm/bridge/adv7511/Kconfig
+++ b/drivers/gpu/drm/bridge/adv7511/Kconfig
@@ -3,6 +3,7 @@ config DRM_I2C_ADV7511
 	depends on OF
 	select DRM_KMS_HELPER
 	select REGMAP_I2C
+	select SND_SOC_HDMI_CODEC if SND_SOC
 	help
 	  Support for the Analog Device ADV7511(W) and ADV7513 HDMI encoders.
 
diff --git a/drivers/gpu/drm/bridge/adv7511/Makefile b/drivers/gpu/drm/bridge/adv7511/Makefile
index 9019327..ad7245d 100644
--- a/drivers/gpu/drm/bridge/adv7511/Makefile
+++ b/drivers/gpu/drm/bridge/adv7511/Makefile
@@ -1,3 +1,3 @@
-adv7511-y := adv7511_drv.o
+adv7511-y := adv7511_drv.o adv7511_audio.o
 adv7511-$(CONFIG_DRM_I2C_ADV7533) += adv7533.o
 obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511.o
diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511.h b/drivers/gpu/drm/bridge/adv7511/adv7511.h
index c7002a0..2e4d340 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511.h
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511.h
@@ -23,6 +23,8 @@ struct adv7511;
 
 int adv7511_packet_enable(struct adv7511 *adv7511, unsigned int packet);
 int adv7511_packet_disable(struct adv7511 *adv7511, unsigned int packet);
+int adv7511_audio_init(struct device *dev);
+void adv7511_audio_exit(struct device *dev);
 
 #define ADV7511_REG_CHIP_REVISION		0x00
 #define ADV7511_REG_N0				0x01
@@ -317,6 +319,8 @@ struct adv7511 {
 	struct drm_display_mode curr_mode;
 
 	unsigned int f_tmds;
+	unsigned int f_audio;
+	unsigned int audio_source;
 
 	unsigned int current_edid_segment;
 	uint8_t edid_buf[256];
@@ -342,6 +346,7 @@ struct adv7511 {
 	bool use_timing_gen;
 
 	enum adv7511_type type;
+	struct platform_device *audio_pdev;
 };
 
 #ifdef CONFIG_DRM_I2C_ADV7533
diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_audio.c b/drivers/gpu/drm/bridge/adv7511/adv7511_audio.c
new file mode 100644
index 0000000..0e0ea6b
--- /dev/null
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511_audio.c
@@ -0,0 +1,199 @@
+/*
+ * Analog Devices ADV7511 HDMI transmitter driver
+ *
+ * Copyright 2012 Analog Devices Inc.
+ * Copyright (c) 2016, Linaro Limited
+ *
+ * Licensed under the GPL-2.
+ */
+
+#include <sound/core.h>
+#include <sound/hdmi-codec.h>
+#include <sound/pcm.h>
+#include <sound/soc.h>
+
+#include "adv7511.h"
+
+static void adv7511_calc_cts_n(unsigned int f_tmds, unsigned int fs,
+			       unsigned int *cts, unsigned int *n)
+{
+	switch (fs) {
+	case 32000:
+		*n = 4096;
+		break;
+	case 44100:
+		*n = 6272;
+		break;
+	case 48000:
+		*n = 6144;
+		break;
+	}
+
+	*cts = ((f_tmds * *n) / (128 * fs)) * 1000;
+}
+
+static int adv7511_update_cts_n(struct adv7511 *adv7511)
+{
+	unsigned int cts = 0;
+	unsigned int n = 0;
+
+	adv7511_calc_cts_n(adv7511->f_tmds, adv7511->f_audio, &cts, &n);
+
+	regmap_write(adv7511->regmap, ADV7511_REG_N0, (n >> 16) & 0xf);
+	regmap_write(adv7511->regmap, ADV7511_REG_N1, (n >> 8) & 0xff);
+	regmap_write(adv7511->regmap, ADV7511_REG_N2, n & 0xff);
+
+	regmap_write(adv7511->regmap, ADV7511_REG_CTS_MANUAL0,
+		     (cts >> 16) & 0xf);
+	regmap_write(adv7511->regmap, ADV7511_REG_CTS_MANUAL1,
+		     (cts >> 8) & 0xff);
+	regmap_write(adv7511->regmap, ADV7511_REG_CTS_MANUAL2,
+		     cts & 0xff);
+
+	return 0;
+}
+
+int adv7511_hdmi_hw_params(struct device *dev, void *data,
+				struct hdmi_codec_daifmt *fmt,
+				struct hdmi_codec_params *hparms)
+{
+	struct adv7511 *adv7511 = dev_get_drvdata(dev);
+	unsigned int audio_source, i2s_format = 0;
+	unsigned int invert_clock;
+	unsigned int rate;
+	unsigned int len;
+
+	switch (hparms->sample_rate) {
+	case 32000:
+		rate = ADV7511_SAMPLE_FREQ_32000;
+		break;
+	case 44100:
+		rate = ADV7511_SAMPLE_FREQ_44100;
+		break;
+	case 48000:
+		rate = ADV7511_SAMPLE_FREQ_48000;
+		break;
+	case 88200:
+		rate = ADV7511_SAMPLE_FREQ_88200;
+		break;
+	case 96000:
+		rate = ADV7511_SAMPLE_FREQ_96000;
+		break;
+	case 176400:
+		rate = ADV7511_SAMPLE_FREQ_176400;
+		break;
+	case 192000:
+		rate = ADV7511_SAMPLE_FREQ_192000;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	switch (hparms->sample_width) {
+	case 16:
+		len = ADV7511_I2S_SAMPLE_LEN_16;
+		break;
+	case 18:
+		len = ADV7511_I2S_SAMPLE_LEN_18;
+		break;
+	case 20:
+		len = ADV7511_I2S_SAMPLE_LEN_20;
+		break;
+	case 24:
+		len = ADV7511_I2S_SAMPLE_LEN_24;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	switch (fmt->fmt) {
+	case HDMI_I2S:
+		audio_source = ADV7511_AUDIO_SOURCE_I2S;
+		i2s_format = ADV7511_I2S_FORMAT_I2S;
+		break;
+	case HDMI_RIGHT_J:
+		audio_source = ADV7511_AUDIO_SOURCE_I2S;
+		i2s_format = ADV7511_I2S_FORMAT_RIGHT_J;
+		break;
+	case HDMI_LEFT_J:
+		audio_source = ADV7511_AUDIO_SOURCE_I2S;
+		i2s_format = ADV7511_I2S_FORMAT_LEFT_J;
+		break;
+//	case HDMI_SPDIF:
+//		audio_source = ADV7511_AUDIO_SOURCE_SPDIF;
+//		break;
+	default:
+		return -EINVAL;
+	}
+
+	invert_clock = fmt->bit_clk_inv;
+
+	regmap_update_bits(adv7511->regmap, ADV7511_REG_AUDIO_SOURCE, 0x70,
+			   audio_source << 4);
+	regmap_update_bits(adv7511->regmap, ADV7511_REG_AUDIO_CONFIG, BIT(6),
+			   invert_clock << 6);
+	regmap_update_bits(adv7511->regmap, ADV7511_REG_I2S_CONFIG, 0x03,
+			   i2s_format);
+
+	adv7511->audio_source = audio_source;
+
+	adv7511->f_audio = hparms->sample_rate;
+
+	adv7511_update_cts_n(adv7511);
+
+	regmap_update_bits(adv7511->regmap, ADV7511_REG_AUDIO_CFG3,
+			   ADV7511_AUDIO_CFG3_LEN_MASK, len);
+	regmap_update_bits(adv7511->regmap, ADV7511_REG_I2C_FREQ_ID_CFG,
+			   ADV7511_I2C_FREQ_ID_CFG_RATE_MASK, rate << 4);
+	regmap_write(adv7511->regmap, 0x73, 0x1);
+
+	return 0;
+}
+
+static int audio_startup(struct device *dev, void *data)
+{
+	struct adv7511 *adv7511 = dev_get_drvdata(dev);
+
+	regmap_update_bits(adv7511->regmap, ADV7511_REG_AUDIO_CONFIG,
+				BIT(7), 0);
+	return 0;
+}
+
+static void audio_shutdown(struct device *dev, void *data)
+{
+
+}
+
+static const struct hdmi_codec_ops adv7511_codec_ops = {
+	.hw_params	= adv7511_hdmi_hw_params,
+	.audio_shutdown = audio_shutdown,
+	.audio_startup	= audio_startup,
+};
+
+static struct hdmi_codec_pdata codec_data = {
+	.ops = &adv7511_codec_ops,
+	.max_i2s_channels = 2,
+	.i2s = 1,
+};
+
+int adv7511_audio_init(struct device *dev)
+{
+	struct adv7511 *adv7511 = dev_get_drvdata(dev);
+
+	adv7511->audio_pdev = platform_device_register_data(dev,
+					HDMI_CODEC_DRV_NAME,
+					PLATFORM_DEVID_AUTO,
+					&codec_data,
+					sizeof(codec_data));
+	return PTR_ERR_OR_ZERO(adv7511->audio_pdev);
+}
+
+void adv7511_audio_exit(struct device *dev)
+{
+	struct adv7511 *adv7511 = dev_get_drvdata(dev);
+
+	if (adv7511->audio_pdev) {
+		platform_device_unregister(adv7511->audio_pdev);
+		adv7511->audio_pdev = NULL;
+	}
+}
diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
index f8eb7f8..3756994 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
@@ -1037,6 +1037,9 @@ static int adv7511_probe(struct i2c_client *i2c, const struct i2c_device_id *id)
 		goto err_unregister_cec;
 	}
 
+	dev_set_drvdata(dev, adv7511);
+	adv7511_audio_init(dev);
+
 	return 0;
 
 err_unregister_cec:
@@ -1058,6 +1061,8 @@ static int adv7511_remove(struct i2c_client *i2c)
 
 	drm_bridge_remove(&adv7511->bridge);
 
+	adv7511_audio_exit(&i2c->dev);
+
 	i2c_unregister_device(adv7511->i2c_edid);
 
 	kfree(adv7511->edid);
-- 
1.9.1

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

* [PATCH 3/4 v2] drm/bridge: adv7511: Enable the audio data and clock pads on adv7533
  2016-08-29 23:41 ` John Stultz
@ 2016-08-29 23:41   ` John Stultz
  -1 siblings, 0 replies; 26+ messages in thread
From: John Stultz @ 2016-08-29 23:41 UTC (permalink / raw)
  To: lkml
  Cc: Srinivas Kandagatla, David Airlie, Archit Taneja,
	Laurent Pinchart, Wolfram Sang, Ville Syrjälä,
	Boris Brezillon, Andy Green, Dave Long, Guodong Xu, Zhangfei Gao,
	Mark Brown, Lars-Peter Clausen, Jose Abreu, dri-devel,
	John Stultz

From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

This patch enables the Audio Data and Clock pads to the adv7533 bridge.
Without this patch audio can not be played.

Cc: David Airlie <airlied@linux.ie>
Cc: Archit Taneja <architt@codeaurora.org>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
Cc: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
Cc: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
Cc: Boris Brezillon <boris.brezillon@free-electrons.com>
Cc: Andy Green <andy@warmcat.com>
Cc: Dave Long <dave.long@linaro.org>
Cc: Guodong Xu <guodong.xu@linaro.org>
Cc: Zhangfei Gao <zhangfei.gao@linaro.org>
Cc: Mark Brown <broonie@kernel.org>
Cc: Lars-Peter Clausen <lars@metafoo.de>
Cc: Jose Abreu <joabreu@synopsys.com>
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 drivers/gpu/drm/bridge/adv7511/adv7533.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/bridge/adv7511/adv7533.c b/drivers/gpu/drm/bridge/adv7511/adv7533.c
index 5eebd15..6798ecf 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7533.c
+++ b/drivers/gpu/drm/bridge/adv7511/adv7533.c
@@ -29,6 +29,7 @@ static const struct reg_sequence adv7533_cec_fixed_registers[] = {
 	{ 0x17, 0xd0 },
 	{ 0x24, 0x20 },
 	{ 0x57, 0x11 },
+	{ 0x05, 0xc8 },
 };
 
 static const struct regmap_config adv7533_cec_regmap_config = {
-- 
1.9.1

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

* [PATCH 3/4 v2] drm/bridge: adv7511: Enable the audio data and clock pads on adv7533
@ 2016-08-29 23:41   ` John Stultz
  0 siblings, 0 replies; 26+ messages in thread
From: John Stultz @ 2016-08-29 23:41 UTC (permalink / raw)
  To: lkml
  Cc: Guodong Xu, dri-devel, Jose Abreu, Wolfram Sang, Mark Brown,
	Srinivas Kandagatla, Laurent Pinchart, Andy Green, Zhangfei Gao,
	Dave Long

From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

This patch enables the Audio Data and Clock pads to the adv7533 bridge.
Without this patch audio can not be played.

Cc: David Airlie <airlied@linux.ie>
Cc: Archit Taneja <architt@codeaurora.org>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
Cc: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
Cc: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
Cc: Boris Brezillon <boris.brezillon@free-electrons.com>
Cc: Andy Green <andy@warmcat.com>
Cc: Dave Long <dave.long@linaro.org>
Cc: Guodong Xu <guodong.xu@linaro.org>
Cc: Zhangfei Gao <zhangfei.gao@linaro.org>
Cc: Mark Brown <broonie@kernel.org>
Cc: Lars-Peter Clausen <lars@metafoo.de>
Cc: Jose Abreu <joabreu@synopsys.com>
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 drivers/gpu/drm/bridge/adv7511/adv7533.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/bridge/adv7511/adv7533.c b/drivers/gpu/drm/bridge/adv7511/adv7533.c
index 5eebd15..6798ecf 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7533.c
+++ b/drivers/gpu/drm/bridge/adv7511/adv7533.c
@@ -29,6 +29,7 @@ static const struct reg_sequence adv7533_cec_fixed_registers[] = {
 	{ 0x17, 0xd0 },
 	{ 0x24, 0x20 },
 	{ 0x57, 0x11 },
+	{ 0x05, 0xc8 },
 };
 
 static const struct regmap_config adv7533_cec_regmap_config = {
-- 
1.9.1

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

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

* [PATCH 4/4 v2] drm/bridge: adv7511: Initialize audio packet on adv7533
  2016-08-29 23:41 ` John Stultz
                   ` (3 preceding siblings ...)
  (?)
@ 2016-08-29 23:41 ` John Stultz
  2016-09-02  9:49     ` Archit Taneja
  -1 siblings, 1 reply; 26+ messages in thread
From: John Stultz @ 2016-08-29 23:41 UTC (permalink / raw)
  To: lkml
  Cc: Andy Green, David Airlie, Archit Taneja, Laurent Pinchart,
	Wolfram Sang, Srinivas Kandagatla, Ville Syrjälä,
	Boris Brezillon, Andy Green, Dave Long, Guodong Xu, Zhangfei Gao,
	Mark Brown, Lars-Peter Clausen, Jose Abreu, dri-devel,
	John Stultz

From: Andy Green <andy.green@linaro.org>

Set the initial audio packet settings to allow the audio
driver to work.

Cc: David Airlie <airlied@linux.ie>
Cc: Archit Taneja <architt@codeaurora.org>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
Cc: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
Cc: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
Cc: Boris Brezillon <boris.brezillon@free-electrons.com>
Cc: Andy Green <andy@warmcat.com>
Cc: Dave Long <dave.long@linaro.org>
Cc: Guodong Xu <guodong.xu@linaro.org>
Cc: Zhangfei Gao <zhangfei.gao@linaro.org>
Cc: Mark Brown <broonie@kernel.org>
Cc: Lars-Peter Clausen <lars@metafoo.de>
Cc: Jose Abreu <joabreu@synopsys.com>
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Andy Green <andy.green@linaro.org>
[jstultz: Forward ported to mainline, changed to use register
 names rather then hex values, and removed config values set by
 audio driver.]
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 drivers/gpu/drm/bridge/adv7511/adv7533.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/drivers/gpu/drm/bridge/adv7511/adv7533.c b/drivers/gpu/drm/bridge/adv7511/adv7533.c
index 6798ecf..cced7c9 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7533.c
+++ b/drivers/gpu/drm/bridge/adv7511/adv7533.c
@@ -104,6 +104,28 @@ void adv7533_dsi_power_on(struct adv7511 *adv)
 	/* disable test mode */
 	regmap_write(adv->regmap_cec, 0x55, 0x00);
 
+	/* hide Audio infoframe updates */
+	regmap_update_bits(adv->regmap, ADV7511_REG_INFOFRAME_UPDATE,
+				BIT(5), BIT(5));
+	/* enable N/CTS, enable Audio sample packets */
+	regmap_update_bits(adv->regmap, ADV7511_REG_PACKET_ENABLE1,
+				BIT(5), BIT(5));
+	/* enable N/CTS */
+	regmap_update_bits(adv->regmap, ADV7511_REG_PACKET_ENABLE1,
+				BIT(6), BIT(6));
+	/* not copyrighted */
+	regmap_update_bits(adv->regmap, ADV7511_REG_AUDIO_CFG1,
+				BIT(5), BIT(5));
+	/* enable audio infoframes */
+	regmap_update_bits(adv->regmap, ADV7511_REG_PACKET_ENABLE1,
+				BIT(3), BIT(3));
+	/* AV mute disable */
+	regmap_update_bits(adv->regmap, ADV7511_REG_GC(0),
+				BIT(7) | BIT(6), BIT(7));
+	/* use Audio infoframe updated info */
+	regmap_update_bits(adv->regmap, ADV7511_REG_GC(1),
+				BIT(5), 0);
+
 	regmap_register_patch(adv->regmap_cec, adv7533_cec_fixed_registers,
 			      ARRAY_SIZE(adv7533_cec_fixed_registers));
 }
-- 
1.9.1

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

* Re: [PATCH 1/4 v2] drm/bridge: adv7511: Move the common data structures to header file
  2016-08-29 23:41   ` John Stultz
@ 2016-08-30  8:56     ` Laurent Pinchart
  -1 siblings, 0 replies; 26+ messages in thread
From: Laurent Pinchart @ 2016-08-30  8:56 UTC (permalink / raw)
  To: John Stultz
  Cc: lkml, Archit Taneja, David Airlie, Wolfram Sang,
	Srinivas Kandagatla, Ville Syrjälä,
	Boris Brezillon, Andy Green, Dave Long, Guodong Xu, Zhangfei Gao,
	Mark Brown, Lars-Peter Clausen, Jose Abreu, dri-devel

Hi John,

Thank you for the patch.

On Monday 29 Aug 2016 16:41:33 John Stultz wrote:
> From: Archit Taneja <architt@codeaurora.org>
> 
> This patch moves the adv7511 data structure to header file so that the
> audio driver file could use it.

Actually it doesn't, the data structure is already in the header file.

> Cc: David Airlie <airlied@linux.ie>
> Cc: Archit Taneja <architt@codeaurora.org>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
> Cc: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> Cc: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
> Cc: Boris Brezillon <boris.brezillon@free-electrons.com>
> Cc: Andy Green <andy@warmcat.com>
> Cc: Dave Long <dave.long@linaro.org>
> Cc: Guodong Xu <guodong.xu@linaro.org>
> Cc: Zhangfei Gao <zhangfei.gao@linaro.org>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: Lars-Peter Clausen <lars@metafoo.de>
> Cc: Jose Abreu <joabreu@synopsys.com>
> Cc: dri-devel@lists.freedesktop.org
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> Signed-off-by: John Stultz <john.stultz@linaro.org>
> ---
>  drivers/gpu/drm/bridge/adv7511/adv7511.h     | 8 ++++++++
>  drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 4 ++--
>  2 files changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511.h
> b/drivers/gpu/drm/bridge/adv7511/adv7511.h index 161c923..c7002a0 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7511.h
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511.h
> @@ -16,6 +16,14 @@
>  #include <drm/drm_crtc_helper.h>
>  #include <drm/drm_mipi_dsi.h>
> 
> +#include <drm/drm_crtc_helper.h>

Isn't it enough to include that header once ? :-)

> +
> +struct regmap;

This isn't needed, the header includes linux/regmap.h.

> +struct adv7511;
> +
> +int adv7511_packet_enable(struct adv7511 *adv7511, unsigned int packet);
> +int adv7511_packet_disable(struct adv7511 *adv7511, unsigned int packet);

You can move those two functions at the end, with all the other function 
declarations, and get rid of the forward declaration of struct adv7511.

>  #define ADV7511_REG_CHIP_REVISION		0x00
>  #define ADV7511_REG_N0				0x01
>  #define ADV7511_REG_N1				0x02
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c index ec8fb2e..f8eb7f8
> 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> @@ -160,7 +160,7 @@ static void adv7511_set_colormap(struct adv7511
> *adv7511, bool enable, ADV7511_CSC_UPDATE_MODE, 0);
>  }
> 
> -static int adv7511_packet_enable(struct adv7511 *adv7511, unsigned int
> packet)
> +int adv7511_packet_enable(struct adv7511 *adv7511, unsigned int packet)
>  {
>  	if (packet & 0xff)
>  		regmap_update_bits(adv7511->regmap, 
ADV7511_REG_PACKET_ENABLE0,
> @@ -175,7 +175,7 @@ static int adv7511_packet_enable(struct adv7511
> *adv7511, unsigned int packet) return 0;
>  }
> 
> -static int adv7511_packet_disable(struct adv7511 *adv7511, unsigned int
> packet)
> +int adv7511_packet_disable(struct adv7511 *adv7511, unsigned int packet)
>  {
>  	if (packet & 0xff)
>  		regmap_update_bits(adv7511->regmap, 
ADV7511_REG_PACKET_ENABLE0,

-- 
Regards,

Laurent Pinchart

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

* Re: [PATCH 1/4 v2] drm/bridge: adv7511: Move the common data structures to header file
@ 2016-08-30  8:56     ` Laurent Pinchart
  0 siblings, 0 replies; 26+ messages in thread
From: Laurent Pinchart @ 2016-08-30  8:56 UTC (permalink / raw)
  To: John Stultz
  Cc: Guodong Xu, dri-devel, lkml, Jose Abreu, Wolfram Sang,
	Mark Brown, Srinivas Kandagatla, Dave Long, Andy Green,
	Zhangfei Gao

Hi John,

Thank you for the patch.

On Monday 29 Aug 2016 16:41:33 John Stultz wrote:
> From: Archit Taneja <architt@codeaurora.org>
> 
> This patch moves the adv7511 data structure to header file so that the
> audio driver file could use it.

Actually it doesn't, the data structure is already in the header file.

> Cc: David Airlie <airlied@linux.ie>
> Cc: Archit Taneja <architt@codeaurora.org>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
> Cc: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> Cc: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
> Cc: Boris Brezillon <boris.brezillon@free-electrons.com>
> Cc: Andy Green <andy@warmcat.com>
> Cc: Dave Long <dave.long@linaro.org>
> Cc: Guodong Xu <guodong.xu@linaro.org>
> Cc: Zhangfei Gao <zhangfei.gao@linaro.org>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: Lars-Peter Clausen <lars@metafoo.de>
> Cc: Jose Abreu <joabreu@synopsys.com>
> Cc: dri-devel@lists.freedesktop.org
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> Signed-off-by: John Stultz <john.stultz@linaro.org>
> ---
>  drivers/gpu/drm/bridge/adv7511/adv7511.h     | 8 ++++++++
>  drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 4 ++--
>  2 files changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511.h
> b/drivers/gpu/drm/bridge/adv7511/adv7511.h index 161c923..c7002a0 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7511.h
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511.h
> @@ -16,6 +16,14 @@
>  #include <drm/drm_crtc_helper.h>
>  #include <drm/drm_mipi_dsi.h>
> 
> +#include <drm/drm_crtc_helper.h>

Isn't it enough to include that header once ? :-)

> +
> +struct regmap;

This isn't needed, the header includes linux/regmap.h.

> +struct adv7511;
> +
> +int adv7511_packet_enable(struct adv7511 *adv7511, unsigned int packet);
> +int adv7511_packet_disable(struct adv7511 *adv7511, unsigned int packet);

You can move those two functions at the end, with all the other function 
declarations, and get rid of the forward declaration of struct adv7511.

>  #define ADV7511_REG_CHIP_REVISION		0x00
>  #define ADV7511_REG_N0				0x01
>  #define ADV7511_REG_N1				0x02
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c index ec8fb2e..f8eb7f8
> 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> @@ -160,7 +160,7 @@ static void adv7511_set_colormap(struct adv7511
> *adv7511, bool enable, ADV7511_CSC_UPDATE_MODE, 0);
>  }
> 
> -static int adv7511_packet_enable(struct adv7511 *adv7511, unsigned int
> packet)
> +int adv7511_packet_enable(struct adv7511 *adv7511, unsigned int packet)
>  {
>  	if (packet & 0xff)
>  		regmap_update_bits(adv7511->regmap, 
ADV7511_REG_PACKET_ENABLE0,
> @@ -175,7 +175,7 @@ static int adv7511_packet_enable(struct adv7511
> *adv7511, unsigned int packet) return 0;
>  }
> 
> -static int adv7511_packet_disable(struct adv7511 *adv7511, unsigned int
> packet)
> +int adv7511_packet_disable(struct adv7511 *adv7511, unsigned int packet)
>  {
>  	if (packet & 0xff)
>  		regmap_update_bits(adv7511->regmap, 
ADV7511_REG_PACKET_ENABLE0,

-- 
Regards,

Laurent Pinchart

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

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

* Re: [PATCH 2/4 v2] drm/bridge: adv7511: Add Audio support.
  2016-08-29 23:41 ` [PATCH 2/4 v2] drm/bridge: adv7511: Add Audio support John Stultz
@ 2016-08-30  9:21     ` Laurent Pinchart
  0 siblings, 0 replies; 26+ messages in thread
From: Laurent Pinchart @ 2016-08-30  9:21 UTC (permalink / raw)
  To: John Stultz
  Cc: lkml, David Airlie, Archit Taneja, Wolfram Sang,
	Srinivas Kandagatla, Ville Syrjälä,
	Boris Brezillon, Andy Green, Dave Long, Guodong Xu, Zhangfei Gao,
	Mark Brown, Lars-Peter Clausen, Jose Abreu, dri-devel

Hi John,

Thank you for the patch.

On Monday 29 Aug 2016 16:41:34 John Stultz wrote:
> This patch adds support to Audio for both adv7511 and adv7533
> bridge chips.
> 
> This patch was originally from [1] by Lars-Peter Clausen <lars@metafoo.de>
> and was adapted by Archit Taneja <architt@codeaurora.org> and
> Srinivas Kandagatla <srinivas.kandagatla@linaro.org>.
> 
> Then I heavily reworked it to use the hdmi-codec driver. So credit
> to them, but blame to me.
> 
> [1]
> https://github.com/analogdevicesinc/linux/blob/xcomm_zynq/drivers/gpu/drm/i
> 2c/adv7511_audio.c
> 
> Cc: David Airlie <airlied@linux.ie>
> Cc: Archit Taneja <architt@codeaurora.org>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
> Cc: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> Cc: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
> Cc: Boris Brezillon <boris.brezillon@free-electrons.com>
> Cc: Andy Green <andy@warmcat.com>
> Cc: Dave Long <dave.long@linaro.org>
> Cc: Guodong Xu <guodong.xu@linaro.org>
> Cc: Zhangfei Gao <zhangfei.gao@linaro.org>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: Lars-Peter Clausen <lars@metafoo.de>
> Cc: Jose Abreu <joabreu@synopsys.com>
> Cc: dri-devel@lists.freedesktop.org
> Signed-off-by: John Stultz <john.stultz@linaro.org>
> ---
> v2: Integrated feedback from Srinivas
> 
>  drivers/gpu/drm/bridge/adv7511/Kconfig         |   1 +
>  drivers/gpu/drm/bridge/adv7511/Makefile        |   2 +-
>  drivers/gpu/drm/bridge/adv7511/adv7511.h       |   5 +
>  drivers/gpu/drm/bridge/adv7511/adv7511_audio.c | 199 ++++++++++++++++++++++
>  drivers/gpu/drm/bridge/adv7511/adv7511_drv.c   |   5 +
>  5 files changed, 211 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/gpu/drm/bridge/adv7511/adv7511_audio.c
> 
> diff --git a/drivers/gpu/drm/bridge/adv7511/Kconfig
> b/drivers/gpu/drm/bridge/adv7511/Kconfig index d2b0499..e3975e9 100644
> --- a/drivers/gpu/drm/bridge/adv7511/Kconfig
> +++ b/drivers/gpu/drm/bridge/adv7511/Kconfig
> @@ -3,6 +3,7 @@ config DRM_I2C_ADV7511
>  	depends on OF
>  	select DRM_KMS_HELPER
>  	select REGMAP_I2C
> +	select SND_SOC_HDMI_CODEC if SND_SOC

Shouldn't sound support be optional ? I can imagine a board that doesn't wire 
up sound to the ADV7511 but still uses SND_SOC for other purposes.

>  	help
>  	  Support for the Analog Device ADV7511(W) and ADV7513 HDMI encoders.
> 
> diff --git a/drivers/gpu/drm/bridge/adv7511/Makefile
> b/drivers/gpu/drm/bridge/adv7511/Makefile index 9019327..ad7245d 100644
> --- a/drivers/gpu/drm/bridge/adv7511/Makefile
> +++ b/drivers/gpu/drm/bridge/adv7511/Makefile
> @@ -1,3 +1,3 @@
> -adv7511-y := adv7511_drv.o
> +adv7511-y := adv7511_drv.o adv7511_audio.o

Could we avoid compiling adv7511_audio.o if sound support isn't desired to 
avoid bloating the kernel ?

>  adv7511-$(CONFIG_DRM_I2C_ADV7533) += adv7533.o
>  obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511.o
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511.h
> b/drivers/gpu/drm/bridge/adv7511/adv7511.h index c7002a0..2e4d340 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7511.h
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511.h
> @@ -23,6 +23,8 @@ struct adv7511;
> 
>  int adv7511_packet_enable(struct adv7511 *adv7511, unsigned int packet);
>  int adv7511_packet_disable(struct adv7511 *adv7511, unsigned int packet);
> +int adv7511_audio_init(struct device *dev);
> +void adv7511_audio_exit(struct device *dev);
> 
>  #define ADV7511_REG_CHIP_REVISION		0x00
>  #define ADV7511_REG_N0				0x01
> @@ -317,6 +319,8 @@ struct adv7511 {
>  	struct drm_display_mode curr_mode;
> 
>  	unsigned int f_tmds;
> +	unsigned int f_audio;
> +	unsigned int audio_source;
> 
>  	unsigned int current_edid_segment;
>  	uint8_t edid_buf[256];
> @@ -342,6 +346,7 @@ struct adv7511 {
>  	bool use_timing_gen;
> 
>  	enum adv7511_type type;
> +	struct platform_device *audio_pdev;
>  };
> 
>  #ifdef CONFIG_DRM_I2C_ADV7533
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_audio.c
> b/drivers/gpu/drm/bridge/adv7511/adv7511_audio.c new file mode 100644
> index 0000000..0e0ea6b
> --- /dev/null
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_audio.c
> @@ -0,0 +1,199 @@
> +/*
> + * Analog Devices ADV7511 HDMI transmitter driver
> + *
> + * Copyright 2012 Analog Devices Inc.
> + * Copyright (c) 2016, Linaro Limited
> + *
> + * Licensed under the GPL-2.
> + */
> +
> +#include <sound/core.h>
> +#include <sound/hdmi-codec.h>
> +#include <sound/pcm.h>
> +#include <sound/soc.h>
> +
> +#include "adv7511.h"
> +
> +static void adv7511_calc_cts_n(unsigned int f_tmds, unsigned int fs,
> +			       unsigned int *cts, unsigned int *n)
> +{
> +	switch (fs) {
> +	case 32000:
> +		*n = 4096;
> +		break;
> +	case 44100:
> +		*n = 6272;
> +		break;
> +	case 48000:
> +		*n = 6144;
> +		break;
> +	}
> +
> +	*cts = ((f_tmds * *n) / (128 * fs)) * 1000;
> +}
> +
> +static int adv7511_update_cts_n(struct adv7511 *adv7511)
> +{
> +	unsigned int cts = 0;
> +	unsigned int n = 0;
> +
> +	adv7511_calc_cts_n(adv7511->f_tmds, adv7511->f_audio, &cts, &n);
> +
> +	regmap_write(adv7511->regmap, ADV7511_REG_N0, (n >> 16) & 0xf);
> +	regmap_write(adv7511->regmap, ADV7511_REG_N1, (n >> 8) & 0xff);
> +	regmap_write(adv7511->regmap, ADV7511_REG_N2, n & 0xff);
> +
> +	regmap_write(adv7511->regmap, ADV7511_REG_CTS_MANUAL0,
> +		     (cts >> 16) & 0xf);
> +	regmap_write(adv7511->regmap, ADV7511_REG_CTS_MANUAL1,
> +		     (cts >> 8) & 0xff);
> +	regmap_write(adv7511->regmap, ADV7511_REG_CTS_MANUAL2,
> +		     cts & 0xff);
> +
> +	return 0;
> +}
> +
> +int adv7511_hdmi_hw_params(struct device *dev, void *data,
> +				struct hdmi_codec_daifmt *fmt,
> +				struct hdmi_codec_params *hparms)
> +{
> +	struct adv7511 *adv7511 = dev_get_drvdata(dev);
> +	unsigned int audio_source, i2s_format = 0;
> +	unsigned int invert_clock;
> +	unsigned int rate;
> +	unsigned int len;
> +
> +	switch (hparms->sample_rate) {
> +	case 32000:
> +		rate = ADV7511_SAMPLE_FREQ_32000;
> +		break;
> +	case 44100:
> +		rate = ADV7511_SAMPLE_FREQ_44100;
> +		break;
> +	case 48000:
> +		rate = ADV7511_SAMPLE_FREQ_48000;
> +		break;
> +	case 88200:
> +		rate = ADV7511_SAMPLE_FREQ_88200;
> +		break;
> +	case 96000:
> +		rate = ADV7511_SAMPLE_FREQ_96000;
> +		break;
> +	case 176400:
> +		rate = ADV7511_SAMPLE_FREQ_176400;
> +		break;
> +	case 192000:
> +		rate = ADV7511_SAMPLE_FREQ_192000;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	switch (hparms->sample_width) {
> +	case 16:
> +		len = ADV7511_I2S_SAMPLE_LEN_16;
> +		break;
> +	case 18:
> +		len = ADV7511_I2S_SAMPLE_LEN_18;
> +		break;
> +	case 20:
> +		len = ADV7511_I2S_SAMPLE_LEN_20;
> +		break;
> +	case 24:
> +		len = ADV7511_I2S_SAMPLE_LEN_24;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	switch (fmt->fmt) {
> +	case HDMI_I2S:
> +		audio_source = ADV7511_AUDIO_SOURCE_I2S;
> +		i2s_format = ADV7511_I2S_FORMAT_I2S;
> +		break;
> +	case HDMI_RIGHT_J:
> +		audio_source = ADV7511_AUDIO_SOURCE_I2S;
> +		i2s_format = ADV7511_I2S_FORMAT_RIGHT_J;
> +		break;
> +	case HDMI_LEFT_J:
> +		audio_source = ADV7511_AUDIO_SOURCE_I2S;
> +		i2s_format = ADV7511_I2S_FORMAT_LEFT_J;
> +		break;
> +//	case HDMI_SPDIF:
> +//		audio_source = ADV7511_AUDIO_SOURCE_SPDIF;
> +//		break;

No commented out code please.

> +	default:
> +		return -EINVAL;
> +	}
> +
> +	invert_clock = fmt->bit_clk_inv;
> +
> +	regmap_update_bits(adv7511->regmap, ADV7511_REG_AUDIO_SOURCE, 0x70,
> +			   audio_source << 4);
> +	regmap_update_bits(adv7511->regmap, ADV7511_REG_AUDIO_CONFIG, BIT(6),
> +			   invert_clock << 6);
> +	regmap_update_bits(adv7511->regmap, ADV7511_REG_I2S_CONFIG, 0x03,
> +			   i2s_format);
> +
> +	adv7511->audio_source = audio_source;
> +
> +	adv7511->f_audio = hparms->sample_rate;
> +
> +	adv7511_update_cts_n(adv7511);
> +
> +	regmap_update_bits(adv7511->regmap, ADV7511_REG_AUDIO_CFG3,
> +			   ADV7511_AUDIO_CFG3_LEN_MASK, len);
> +	regmap_update_bits(adv7511->regmap, ADV7511_REG_I2C_FREQ_ID_CFG,
> +			   ADV7511_I2C_FREQ_ID_CFG_RATE_MASK, rate << 4);
> +	regmap_write(adv7511->regmap, 0x73, 0x1);
> +
> +	return 0;
> +}
> +
> +static int audio_startup(struct device *dev, void *data)
> +{
> +	struct adv7511 *adv7511 = dev_get_drvdata(dev);
> +
> +	regmap_update_bits(adv7511->regmap, ADV7511_REG_AUDIO_CONFIG,
> +				BIT(7), 0);
> +	return 0;
> +}
> +
> +static void audio_shutdown(struct device *dev, void *data)
> +{
> +

No need for a blank line.

> +}
> +
> +static const struct hdmi_codec_ops adv7511_codec_ops = {
> +	.hw_params	= adv7511_hdmi_hw_params,
> +	.audio_shutdown = audio_shutdown,
> +	.audio_startup	= audio_startup,
> +};
> +
> +static struct hdmi_codec_pdata codec_data = {
> +	.ops = &adv7511_codec_ops,
> +	.max_i2s_channels = 2,
> +	.i2s = 1,
> +};
> +
> +int adv7511_audio_init(struct device *dev)
> +{
> +	struct adv7511 *adv7511 = dev_get_drvdata(dev);

Why don't you pass the struct adv7511 pointer directly to this function ?

> +	adv7511->audio_pdev = platform_device_register_data(dev,
> +					HDMI_CODEC_DRV_NAME,
> +					PLATFORM_DEVID_AUTO,
> +					&codec_data,
> +					sizeof(codec_data));
> +	return PTR_ERR_OR_ZERO(adv7511->audio_pdev);
> +}
> +
> +void adv7511_audio_exit(struct device *dev)
> +{
> +	struct adv7511 *adv7511 = dev_get_drvdata(dev);

Same here.

> +
> +	if (adv7511->audio_pdev) {
> +		platform_device_unregister(adv7511->audio_pdev);
> +		adv7511->audio_pdev = NULL;
> +	}
> +}
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c index f8eb7f8..3756994
> 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> @@ -1037,6 +1037,9 @@ static int adv7511_probe(struct i2c_client *i2c, const
> struct i2c_device_id *id) goto err_unregister_cec;
>  	}
> 
> +	dev_set_drvdata(dev, adv7511);

This is already handled by the i2c_set_clientdata() call earlier in this 
function.

> +	adv7511_audio_init(dev);
> +
>  	return 0;
> 
>  err_unregister_cec:
> @@ -1058,6 +1061,8 @@ static int adv7511_remove(struct i2c_client *i2c)
> 
>  	drm_bridge_remove(&adv7511->bridge);
> 
> +	adv7511_audio_exit(&i2c->dev);
> +
>  	i2c_unregister_device(adv7511->i2c_edid);
> 
>  	kfree(adv7511->edid);

-- 
Regards,

Laurent Pinchart

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

* Re: [PATCH 2/4 v2] drm/bridge: adv7511: Add Audio support.
@ 2016-08-30  9:21     ` Laurent Pinchart
  0 siblings, 0 replies; 26+ messages in thread
From: Laurent Pinchart @ 2016-08-30  9:21 UTC (permalink / raw)
  To: John Stultz
  Cc: Guodong Xu, dri-devel, lkml, Jose Abreu, Wolfram Sang,
	Mark Brown, Srinivas Kandagatla, Dave Long, Andy Green,
	Zhangfei Gao

Hi John,

Thank you for the patch.

On Monday 29 Aug 2016 16:41:34 John Stultz wrote:
> This patch adds support to Audio for both adv7511 and adv7533
> bridge chips.
> 
> This patch was originally from [1] by Lars-Peter Clausen <lars@metafoo.de>
> and was adapted by Archit Taneja <architt@codeaurora.org> and
> Srinivas Kandagatla <srinivas.kandagatla@linaro.org>.
> 
> Then I heavily reworked it to use the hdmi-codec driver. So credit
> to them, but blame to me.
> 
> [1]
> https://github.com/analogdevicesinc/linux/blob/xcomm_zynq/drivers/gpu/drm/i
> 2c/adv7511_audio.c
> 
> Cc: David Airlie <airlied@linux.ie>
> Cc: Archit Taneja <architt@codeaurora.org>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
> Cc: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> Cc: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
> Cc: Boris Brezillon <boris.brezillon@free-electrons.com>
> Cc: Andy Green <andy@warmcat.com>
> Cc: Dave Long <dave.long@linaro.org>
> Cc: Guodong Xu <guodong.xu@linaro.org>
> Cc: Zhangfei Gao <zhangfei.gao@linaro.org>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: Lars-Peter Clausen <lars@metafoo.de>
> Cc: Jose Abreu <joabreu@synopsys.com>
> Cc: dri-devel@lists.freedesktop.org
> Signed-off-by: John Stultz <john.stultz@linaro.org>
> ---
> v2: Integrated feedback from Srinivas
> 
>  drivers/gpu/drm/bridge/adv7511/Kconfig         |   1 +
>  drivers/gpu/drm/bridge/adv7511/Makefile        |   2 +-
>  drivers/gpu/drm/bridge/adv7511/adv7511.h       |   5 +
>  drivers/gpu/drm/bridge/adv7511/adv7511_audio.c | 199 ++++++++++++++++++++++
>  drivers/gpu/drm/bridge/adv7511/adv7511_drv.c   |   5 +
>  5 files changed, 211 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/gpu/drm/bridge/adv7511/adv7511_audio.c
> 
> diff --git a/drivers/gpu/drm/bridge/adv7511/Kconfig
> b/drivers/gpu/drm/bridge/adv7511/Kconfig index d2b0499..e3975e9 100644
> --- a/drivers/gpu/drm/bridge/adv7511/Kconfig
> +++ b/drivers/gpu/drm/bridge/adv7511/Kconfig
> @@ -3,6 +3,7 @@ config DRM_I2C_ADV7511
>  	depends on OF
>  	select DRM_KMS_HELPER
>  	select REGMAP_I2C
> +	select SND_SOC_HDMI_CODEC if SND_SOC

Shouldn't sound support be optional ? I can imagine a board that doesn't wire 
up sound to the ADV7511 but still uses SND_SOC for other purposes.

>  	help
>  	  Support for the Analog Device ADV7511(W) and ADV7513 HDMI encoders.
> 
> diff --git a/drivers/gpu/drm/bridge/adv7511/Makefile
> b/drivers/gpu/drm/bridge/adv7511/Makefile index 9019327..ad7245d 100644
> --- a/drivers/gpu/drm/bridge/adv7511/Makefile
> +++ b/drivers/gpu/drm/bridge/adv7511/Makefile
> @@ -1,3 +1,3 @@
> -adv7511-y := adv7511_drv.o
> +adv7511-y := adv7511_drv.o adv7511_audio.o

Could we avoid compiling adv7511_audio.o if sound support isn't desired to 
avoid bloating the kernel ?

>  adv7511-$(CONFIG_DRM_I2C_ADV7533) += adv7533.o
>  obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511.o
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511.h
> b/drivers/gpu/drm/bridge/adv7511/adv7511.h index c7002a0..2e4d340 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7511.h
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511.h
> @@ -23,6 +23,8 @@ struct adv7511;
> 
>  int adv7511_packet_enable(struct adv7511 *adv7511, unsigned int packet);
>  int adv7511_packet_disable(struct adv7511 *adv7511, unsigned int packet);
> +int adv7511_audio_init(struct device *dev);
> +void adv7511_audio_exit(struct device *dev);
> 
>  #define ADV7511_REG_CHIP_REVISION		0x00
>  #define ADV7511_REG_N0				0x01
> @@ -317,6 +319,8 @@ struct adv7511 {
>  	struct drm_display_mode curr_mode;
> 
>  	unsigned int f_tmds;
> +	unsigned int f_audio;
> +	unsigned int audio_source;
> 
>  	unsigned int current_edid_segment;
>  	uint8_t edid_buf[256];
> @@ -342,6 +346,7 @@ struct adv7511 {
>  	bool use_timing_gen;
> 
>  	enum adv7511_type type;
> +	struct platform_device *audio_pdev;
>  };
> 
>  #ifdef CONFIG_DRM_I2C_ADV7533
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_audio.c
> b/drivers/gpu/drm/bridge/adv7511/adv7511_audio.c new file mode 100644
> index 0000000..0e0ea6b
> --- /dev/null
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_audio.c
> @@ -0,0 +1,199 @@
> +/*
> + * Analog Devices ADV7511 HDMI transmitter driver
> + *
> + * Copyright 2012 Analog Devices Inc.
> + * Copyright (c) 2016, Linaro Limited
> + *
> + * Licensed under the GPL-2.
> + */
> +
> +#include <sound/core.h>
> +#include <sound/hdmi-codec.h>
> +#include <sound/pcm.h>
> +#include <sound/soc.h>
> +
> +#include "adv7511.h"
> +
> +static void adv7511_calc_cts_n(unsigned int f_tmds, unsigned int fs,
> +			       unsigned int *cts, unsigned int *n)
> +{
> +	switch (fs) {
> +	case 32000:
> +		*n = 4096;
> +		break;
> +	case 44100:
> +		*n = 6272;
> +		break;
> +	case 48000:
> +		*n = 6144;
> +		break;
> +	}
> +
> +	*cts = ((f_tmds * *n) / (128 * fs)) * 1000;
> +}
> +
> +static int adv7511_update_cts_n(struct adv7511 *adv7511)
> +{
> +	unsigned int cts = 0;
> +	unsigned int n = 0;
> +
> +	adv7511_calc_cts_n(adv7511->f_tmds, adv7511->f_audio, &cts, &n);
> +
> +	regmap_write(adv7511->regmap, ADV7511_REG_N0, (n >> 16) & 0xf);
> +	regmap_write(adv7511->regmap, ADV7511_REG_N1, (n >> 8) & 0xff);
> +	regmap_write(adv7511->regmap, ADV7511_REG_N2, n & 0xff);
> +
> +	regmap_write(adv7511->regmap, ADV7511_REG_CTS_MANUAL0,
> +		     (cts >> 16) & 0xf);
> +	regmap_write(adv7511->regmap, ADV7511_REG_CTS_MANUAL1,
> +		     (cts >> 8) & 0xff);
> +	regmap_write(adv7511->regmap, ADV7511_REG_CTS_MANUAL2,
> +		     cts & 0xff);
> +
> +	return 0;
> +}
> +
> +int adv7511_hdmi_hw_params(struct device *dev, void *data,
> +				struct hdmi_codec_daifmt *fmt,
> +				struct hdmi_codec_params *hparms)
> +{
> +	struct adv7511 *adv7511 = dev_get_drvdata(dev);
> +	unsigned int audio_source, i2s_format = 0;
> +	unsigned int invert_clock;
> +	unsigned int rate;
> +	unsigned int len;
> +
> +	switch (hparms->sample_rate) {
> +	case 32000:
> +		rate = ADV7511_SAMPLE_FREQ_32000;
> +		break;
> +	case 44100:
> +		rate = ADV7511_SAMPLE_FREQ_44100;
> +		break;
> +	case 48000:
> +		rate = ADV7511_SAMPLE_FREQ_48000;
> +		break;
> +	case 88200:
> +		rate = ADV7511_SAMPLE_FREQ_88200;
> +		break;
> +	case 96000:
> +		rate = ADV7511_SAMPLE_FREQ_96000;
> +		break;
> +	case 176400:
> +		rate = ADV7511_SAMPLE_FREQ_176400;
> +		break;
> +	case 192000:
> +		rate = ADV7511_SAMPLE_FREQ_192000;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	switch (hparms->sample_width) {
> +	case 16:
> +		len = ADV7511_I2S_SAMPLE_LEN_16;
> +		break;
> +	case 18:
> +		len = ADV7511_I2S_SAMPLE_LEN_18;
> +		break;
> +	case 20:
> +		len = ADV7511_I2S_SAMPLE_LEN_20;
> +		break;
> +	case 24:
> +		len = ADV7511_I2S_SAMPLE_LEN_24;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	switch (fmt->fmt) {
> +	case HDMI_I2S:
> +		audio_source = ADV7511_AUDIO_SOURCE_I2S;
> +		i2s_format = ADV7511_I2S_FORMAT_I2S;
> +		break;
> +	case HDMI_RIGHT_J:
> +		audio_source = ADV7511_AUDIO_SOURCE_I2S;
> +		i2s_format = ADV7511_I2S_FORMAT_RIGHT_J;
> +		break;
> +	case HDMI_LEFT_J:
> +		audio_source = ADV7511_AUDIO_SOURCE_I2S;
> +		i2s_format = ADV7511_I2S_FORMAT_LEFT_J;
> +		break;
> +//	case HDMI_SPDIF:
> +//		audio_source = ADV7511_AUDIO_SOURCE_SPDIF;
> +//		break;

No commented out code please.

> +	default:
> +		return -EINVAL;
> +	}
> +
> +	invert_clock = fmt->bit_clk_inv;
> +
> +	regmap_update_bits(adv7511->regmap, ADV7511_REG_AUDIO_SOURCE, 0x70,
> +			   audio_source << 4);
> +	regmap_update_bits(adv7511->regmap, ADV7511_REG_AUDIO_CONFIG, BIT(6),
> +			   invert_clock << 6);
> +	regmap_update_bits(adv7511->regmap, ADV7511_REG_I2S_CONFIG, 0x03,
> +			   i2s_format);
> +
> +	adv7511->audio_source = audio_source;
> +
> +	adv7511->f_audio = hparms->sample_rate;
> +
> +	adv7511_update_cts_n(adv7511);
> +
> +	regmap_update_bits(adv7511->regmap, ADV7511_REG_AUDIO_CFG3,
> +			   ADV7511_AUDIO_CFG3_LEN_MASK, len);
> +	regmap_update_bits(adv7511->regmap, ADV7511_REG_I2C_FREQ_ID_CFG,
> +			   ADV7511_I2C_FREQ_ID_CFG_RATE_MASK, rate << 4);
> +	regmap_write(adv7511->regmap, 0x73, 0x1);
> +
> +	return 0;
> +}
> +
> +static int audio_startup(struct device *dev, void *data)
> +{
> +	struct adv7511 *adv7511 = dev_get_drvdata(dev);
> +
> +	regmap_update_bits(adv7511->regmap, ADV7511_REG_AUDIO_CONFIG,
> +				BIT(7), 0);
> +	return 0;
> +}
> +
> +static void audio_shutdown(struct device *dev, void *data)
> +{
> +

No need for a blank line.

> +}
> +
> +static const struct hdmi_codec_ops adv7511_codec_ops = {
> +	.hw_params	= adv7511_hdmi_hw_params,
> +	.audio_shutdown = audio_shutdown,
> +	.audio_startup	= audio_startup,
> +};
> +
> +static struct hdmi_codec_pdata codec_data = {
> +	.ops = &adv7511_codec_ops,
> +	.max_i2s_channels = 2,
> +	.i2s = 1,
> +};
> +
> +int adv7511_audio_init(struct device *dev)
> +{
> +	struct adv7511 *adv7511 = dev_get_drvdata(dev);

Why don't you pass the struct adv7511 pointer directly to this function ?

> +	adv7511->audio_pdev = platform_device_register_data(dev,
> +					HDMI_CODEC_DRV_NAME,
> +					PLATFORM_DEVID_AUTO,
> +					&codec_data,
> +					sizeof(codec_data));
> +	return PTR_ERR_OR_ZERO(adv7511->audio_pdev);
> +}
> +
> +void adv7511_audio_exit(struct device *dev)
> +{
> +	struct adv7511 *adv7511 = dev_get_drvdata(dev);

Same here.

> +
> +	if (adv7511->audio_pdev) {
> +		platform_device_unregister(adv7511->audio_pdev);
> +		adv7511->audio_pdev = NULL;
> +	}
> +}
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c index f8eb7f8..3756994
> 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> @@ -1037,6 +1037,9 @@ static int adv7511_probe(struct i2c_client *i2c, const
> struct i2c_device_id *id) goto err_unregister_cec;
>  	}
> 
> +	dev_set_drvdata(dev, adv7511);

This is already handled by the i2c_set_clientdata() call earlier in this 
function.

> +	adv7511_audio_init(dev);
> +
>  	return 0;
> 
>  err_unregister_cec:
> @@ -1058,6 +1061,8 @@ static int adv7511_remove(struct i2c_client *i2c)
> 
>  	drm_bridge_remove(&adv7511->bridge);
> 
> +	adv7511_audio_exit(&i2c->dev);
> +
>  	i2c_unregister_device(adv7511->i2c_edid);
> 
>  	kfree(adv7511->edid);

-- 
Regards,

Laurent Pinchart

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

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

* Re: [PATCH 0/4 v2] Audio support for adv7511 hdmi bridge
  2016-08-29 23:41 ` John Stultz
@ 2016-08-30  9:23   ` Laurent Pinchart
  -1 siblings, 0 replies; 26+ messages in thread
From: Laurent Pinchart @ 2016-08-30  9:23 UTC (permalink / raw)
  To: John Stultz
  Cc: lkml, David Airlie, Archit Taneja, Wolfram Sang,
	Srinivas Kandagatla, Ville Syrjälä,
	Boris Brezillon, Andy Green, Dave Long, Guodong Xu, Zhangfei Gao,
	Mark Brown, Lars-Peter Clausen, Jose Abreu, dri-devel

Hi John,

Thank you for the patches.

On Monday 29 Aug 2016 16:41:32 John Stultz wrote:
> This is another swing at getting the adv7511 hdmi bridge
> audio support reviewed.
> 
> I've taken the core audio work done by Lars-Peter Clausen, and
> adapted by Srinivas Kandagatla and Archit Taneja, and tried to
> rework it to use the hdmi-codec sound driver.
> 
> This patchset, along with the i2s driver and dts changes allows
> HDMI audio to work on the HiKey board.

Where are the dts changes ?

> I'd really appreciate any thoughts or feedback.
> 
> New in v2:
> * Integrated Srinivas' review feedback
> 
> thanks
> -john
> 
> Cc: David Airlie <airlied@linux.ie>
> Cc: Archit Taneja <architt@codeaurora.org>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
> Cc: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> Cc: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
> Cc: Boris Brezillon <boris.brezillon@free-electrons.com>
> Cc: Andy Green <andy@warmcat.com>
> Cc: Dave Long <dave.long@linaro.org>
> Cc: Guodong Xu <guodong.xu@linaro.org>
> Cc: Zhangfei Gao <zhangfei.gao@linaro.org>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: Lars-Peter Clausen <lars@metafoo.de>
> Cc: Jose Abreu <joabreu@synopsys.com>
> Cc: dri-devel@lists.freedesktop.org
> 
> Andy Green (1):
>   drm/bridge: adv7511: Initialize audio packet on adv7533
> 
> Archit Taneja (1):
>   drm/bridge: adv7511: Move the common data structures to header file
> 
> John Stultz (1):
>   drm/bridge: adv7511: Add Audio support.
> 
> Srinivas Kandagatla (1):
>   drm/bridge: adv7511: Enable the audio data and clock pads on adv7533
> 
>  drivers/gpu/drm/bridge/adv7511/Kconfig         |   1 +
>  drivers/gpu/drm/bridge/adv7511/Makefile        |   2 +-
>  drivers/gpu/drm/bridge/adv7511/adv7511.h       |  13 ++
>  drivers/gpu/drm/bridge/adv7511/adv7511_audio.c | 199 ++++++++++++++++++++++
>  drivers/gpu/drm/bridge/adv7511/adv7511_drv.c   |   9 +-
>  drivers/gpu/drm/bridge/adv7511/adv7533.c       |  23 +++
>  6 files changed, 244 insertions(+), 3 deletions(-)
>  create mode 100644 drivers/gpu/drm/bridge/adv7511/adv7511_audio.c

-- 
Regards,

Laurent Pinchart

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

* Re: [PATCH 0/4 v2] Audio support for adv7511 hdmi bridge
@ 2016-08-30  9:23   ` Laurent Pinchart
  0 siblings, 0 replies; 26+ messages in thread
From: Laurent Pinchart @ 2016-08-30  9:23 UTC (permalink / raw)
  To: John Stultz
  Cc: Guodong Xu, dri-devel, lkml, Jose Abreu, Wolfram Sang,
	Mark Brown, Srinivas Kandagatla, Dave Long, Andy Green,
	Zhangfei Gao

Hi John,

Thank you for the patches.

On Monday 29 Aug 2016 16:41:32 John Stultz wrote:
> This is another swing at getting the adv7511 hdmi bridge
> audio support reviewed.
> 
> I've taken the core audio work done by Lars-Peter Clausen, and
> adapted by Srinivas Kandagatla and Archit Taneja, and tried to
> rework it to use the hdmi-codec sound driver.
> 
> This patchset, along with the i2s driver and dts changes allows
> HDMI audio to work on the HiKey board.

Where are the dts changes ?

> I'd really appreciate any thoughts or feedback.
> 
> New in v2:
> * Integrated Srinivas' review feedback
> 
> thanks
> -john
> 
> Cc: David Airlie <airlied@linux.ie>
> Cc: Archit Taneja <architt@codeaurora.org>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
> Cc: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> Cc: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
> Cc: Boris Brezillon <boris.brezillon@free-electrons.com>
> Cc: Andy Green <andy@warmcat.com>
> Cc: Dave Long <dave.long@linaro.org>
> Cc: Guodong Xu <guodong.xu@linaro.org>
> Cc: Zhangfei Gao <zhangfei.gao@linaro.org>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: Lars-Peter Clausen <lars@metafoo.de>
> Cc: Jose Abreu <joabreu@synopsys.com>
> Cc: dri-devel@lists.freedesktop.org
> 
> Andy Green (1):
>   drm/bridge: adv7511: Initialize audio packet on adv7533
> 
> Archit Taneja (1):
>   drm/bridge: adv7511: Move the common data structures to header file
> 
> John Stultz (1):
>   drm/bridge: adv7511: Add Audio support.
> 
> Srinivas Kandagatla (1):
>   drm/bridge: adv7511: Enable the audio data and clock pads on adv7533
> 
>  drivers/gpu/drm/bridge/adv7511/Kconfig         |   1 +
>  drivers/gpu/drm/bridge/adv7511/Makefile        |   2 +-
>  drivers/gpu/drm/bridge/adv7511/adv7511.h       |  13 ++
>  drivers/gpu/drm/bridge/adv7511/adv7511_audio.c | 199 ++++++++++++++++++++++
>  drivers/gpu/drm/bridge/adv7511/adv7511_drv.c   |   9 +-
>  drivers/gpu/drm/bridge/adv7511/adv7533.c       |  23 +++
>  6 files changed, 244 insertions(+), 3 deletions(-)
>  create mode 100644 drivers/gpu/drm/bridge/adv7511/adv7511_audio.c

-- 
Regards,

Laurent Pinchart

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

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

* Re: [PATCH 4/4 v2] drm/bridge: adv7511: Initialize audio packet on adv7533
  2016-08-29 23:41 ` [PATCH 4/4 v2] drm/bridge: adv7511: Initialize audio packet " John Stultz
@ 2016-09-02  9:49     ` Archit Taneja
  0 siblings, 0 replies; 26+ messages in thread
From: Archit Taneja @ 2016-09-02  9:49 UTC (permalink / raw)
  To: John Stultz, lkml
  Cc: Andy Green, David Airlie, Laurent Pinchart, Wolfram Sang,
	Srinivas Kandagatla, Ville Syrjälä,
	Boris Brezillon, Andy Green, Dave Long, Guodong Xu, Zhangfei Gao,
	Mark Brown, Lars-Peter Clausen, Jose Abreu, dri-devel

Hi,

On 8/30/2016 5:11 AM, John Stultz wrote:
> From: Andy Green <andy.green@linaro.org>
>
> Set the initial audio packet settings to allow the audio
> driver to work.
>
> Cc: David Airlie <airlied@linux.ie>
> Cc: Archit Taneja <architt@codeaurora.org>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
> Cc: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> Cc: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
> Cc: Boris Brezillon <boris.brezillon@free-electrons.com>
> Cc: Andy Green <andy@warmcat.com>
> Cc: Dave Long <dave.long@linaro.org>
> Cc: Guodong Xu <guodong.xu@linaro.org>
> Cc: Zhangfei Gao <zhangfei.gao@linaro.org>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: Lars-Peter Clausen <lars@metafoo.de>
> Cc: Jose Abreu <joabreu@synopsys.com>
> Cc: dri-devel@lists.freedesktop.org
> Signed-off-by: Andy Green <andy.green@linaro.org>
> [jstultz: Forward ported to mainline, changed to use register
>   names rather then hex values, and removed config values set by
>   audio driver.]
> Signed-off-by: John Stultz <john.stultz@linaro.org>
> ---
>   drivers/gpu/drm/bridge/adv7511/adv7533.c | 22 ++++++++++++++++++++++
>   1 file changed, 22 insertions(+)
>
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7533.c b/drivers/gpu/drm/bridge/adv7511/adv7533.c
> index 6798ecf..cced7c9 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7533.c
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7533.c
> @@ -104,6 +104,28 @@ void adv7533_dsi_power_on(struct adv7511 *adv)
>   	/* disable test mode */
>   	regmap_write(adv->regmap_cec, 0x55, 0x00);
>
> +	/* hide Audio infoframe updates */
> +	regmap_update_bits(adv->regmap, ADV7511_REG_INFOFRAME_UPDATE,
> +				BIT(5), BIT(5));
> +	/* enable N/CTS, enable Audio sample packets */
> +	regmap_update_bits(adv->regmap, ADV7511_REG_PACKET_ENABLE1,
> +				BIT(5), BIT(5));
> +	/* enable N/CTS */
> +	regmap_update_bits(adv->regmap, ADV7511_REG_PACKET_ENABLE1,
> +				BIT(6), BIT(6));
> +	/* not copyrighted */
> +	regmap_update_bits(adv->regmap, ADV7511_REG_AUDIO_CFG1,
> +				BIT(5), BIT(5));
> +	/* enable audio infoframes */
> +	regmap_update_bits(adv->regmap, ADV7511_REG_PACKET_ENABLE1,
> +				BIT(3), BIT(3));
> +	/* AV mute disable */
> +	regmap_update_bits(adv->regmap, ADV7511_REG_GC(0),
> +				BIT(7) | BIT(6), BIT(7));
> +	/* use Audio infoframe updated info */
> +	regmap_update_bits(adv->regmap, ADV7511_REG_GC(1),
> +				BIT(5), 0);
> +

Wouldn't these writes be needed by ADV751x chips too? These seem to
belong to the main ADV75xx regmap. These should probably be a separate 
func in adv7511_audio.c or adv7511_drv.c, and it should be called in
adv7511_power_on, so that ADV751x chips can utilize this too.

Thanks,
Archit

>   	regmap_register_patch(adv->regmap_cec, adv7533_cec_fixed_registers,
>   			      ARRAY_SIZE(adv7533_cec_fixed_registers));
>   }
>

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

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

* Re: [PATCH 4/4 v2] drm/bridge: adv7511: Initialize audio packet on adv7533
@ 2016-09-02  9:49     ` Archit Taneja
  0 siblings, 0 replies; 26+ messages in thread
From: Archit Taneja @ 2016-09-02  9:49 UTC (permalink / raw)
  To: John Stultz, lkml
  Cc: Andy Green, Guodong Xu, dri-devel, Jose Abreu, Wolfram Sang,
	Mark Brown, Srinivas Kandagatla, Laurent Pinchart, Andy Green,
	Zhangfei Gao, Dave Long

Hi,

On 8/30/2016 5:11 AM, John Stultz wrote:
> From: Andy Green <andy.green@linaro.org>
>
> Set the initial audio packet settings to allow the audio
> driver to work.
>
> Cc: David Airlie <airlied@linux.ie>
> Cc: Archit Taneja <architt@codeaurora.org>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
> Cc: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> Cc: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
> Cc: Boris Brezillon <boris.brezillon@free-electrons.com>
> Cc: Andy Green <andy@warmcat.com>
> Cc: Dave Long <dave.long@linaro.org>
> Cc: Guodong Xu <guodong.xu@linaro.org>
> Cc: Zhangfei Gao <zhangfei.gao@linaro.org>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: Lars-Peter Clausen <lars@metafoo.de>
> Cc: Jose Abreu <joabreu@synopsys.com>
> Cc: dri-devel@lists.freedesktop.org
> Signed-off-by: Andy Green <andy.green@linaro.org>
> [jstultz: Forward ported to mainline, changed to use register
>   names rather then hex values, and removed config values set by
>   audio driver.]
> Signed-off-by: John Stultz <john.stultz@linaro.org>
> ---
>   drivers/gpu/drm/bridge/adv7511/adv7533.c | 22 ++++++++++++++++++++++
>   1 file changed, 22 insertions(+)
>
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7533.c b/drivers/gpu/drm/bridge/adv7511/adv7533.c
> index 6798ecf..cced7c9 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7533.c
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7533.c
> @@ -104,6 +104,28 @@ void adv7533_dsi_power_on(struct adv7511 *adv)
>   	/* disable test mode */
>   	regmap_write(adv->regmap_cec, 0x55, 0x00);
>
> +	/* hide Audio infoframe updates */
> +	regmap_update_bits(adv->regmap, ADV7511_REG_INFOFRAME_UPDATE,
> +				BIT(5), BIT(5));
> +	/* enable N/CTS, enable Audio sample packets */
> +	regmap_update_bits(adv->regmap, ADV7511_REG_PACKET_ENABLE1,
> +				BIT(5), BIT(5));
> +	/* enable N/CTS */
> +	regmap_update_bits(adv->regmap, ADV7511_REG_PACKET_ENABLE1,
> +				BIT(6), BIT(6));
> +	/* not copyrighted */
> +	regmap_update_bits(adv->regmap, ADV7511_REG_AUDIO_CFG1,
> +				BIT(5), BIT(5));
> +	/* enable audio infoframes */
> +	regmap_update_bits(adv->regmap, ADV7511_REG_PACKET_ENABLE1,
> +				BIT(3), BIT(3));
> +	/* AV mute disable */
> +	regmap_update_bits(adv->regmap, ADV7511_REG_GC(0),
> +				BIT(7) | BIT(6), BIT(7));
> +	/* use Audio infoframe updated info */
> +	regmap_update_bits(adv->regmap, ADV7511_REG_GC(1),
> +				BIT(5), 0);
> +

Wouldn't these writes be needed by ADV751x chips too? These seem to
belong to the main ADV75xx regmap. These should probably be a separate 
func in adv7511_audio.c or adv7511_drv.c, and it should be called in
adv7511_power_on, so that ADV751x chips can utilize this too.

Thanks,
Archit

>   	regmap_register_patch(adv->regmap_cec, adv7533_cec_fixed_registers,
>   			      ARRAY_SIZE(adv7533_cec_fixed_registers));
>   }
>

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [PATCH 0/4 v2] Audio support for adv7511 hdmi bridge
  2016-08-30  9:23   ` Laurent Pinchart
  (?)
@ 2016-09-06 22:17   ` John Stultz
  2016-10-18 14:42       ` Laurent Pinchart
  -1 siblings, 1 reply; 26+ messages in thread
From: John Stultz @ 2016-09-06 22:17 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: lkml, David Airlie, Archit Taneja, Wolfram Sang,
	Srinivas Kandagatla, Ville Syrjälä,
	Boris Brezillon, Andy Green, Dave Long, Guodong Xu, Zhangfei Gao,
	Mark Brown, Lars-Peter Clausen, Jose Abreu, dri-devel

On Tue, Aug 30, 2016 at 2:23 AM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> Hi John,
>
> Thank you for the patches.

Thanks so much for the review! I'm reworking the patchset now and will
be sending out an updated set soon!


>> This patchset, along with the i2s driver and dts changes allows
>> HDMI audio to work on the HiKey board.
>
> Where are the dts changes ?

Here's what I'm using to get it working:
https://git.linaro.org/people/john.stultz/android-dev.git/commitdiff/1024cb485b6f00a7e355ce60425f04a584481148

But again, that's dependent on the k3dma driver (queued), the hi6210
i2s driver (still being reworked), and adv7511 audio (this patchset).

thanks
-john

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

* Re: [PATCH 1/4 v2] drm/bridge: adv7511: Move the common data structures to header file
  2016-08-30  8:56     ` Laurent Pinchart
@ 2016-09-06 22:49       ` John Stultz
  -1 siblings, 0 replies; 26+ messages in thread
From: John Stultz @ 2016-09-06 22:49 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: lkml, Archit Taneja, David Airlie, Wolfram Sang,
	Srinivas Kandagatla, Ville Syrjälä,
	Boris Brezillon, Andy Green, Dave Long, Guodong Xu, Zhangfei Gao,
	Mark Brown, Lars-Peter Clausen, Jose Abreu, dri-devel

On Tue, Aug 30, 2016 at 1:56 AM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> Hi John,
>
> Thank you for the patch.
>
> On Monday 29 Aug 2016 16:41:33 John Stultz wrote:
>> From: Archit Taneja <architt@codeaurora.org>
>>
>> This patch moves the adv7511 data structure to header file so that the
>> audio driver file could use it.
>
> Actually it doesn't, the data structure is already in the header file.

Heh.  Yea, it looks like most of that patch has fallen out, and
looking closer at it, the remainders aren't necessary, so I'm dropping
the whole thing.

thanks
-john

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

* Re: [PATCH 1/4 v2] drm/bridge: adv7511: Move the common data structures to header file
@ 2016-09-06 22:49       ` John Stultz
  0 siblings, 0 replies; 26+ messages in thread
From: John Stultz @ 2016-09-06 22:49 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Guodong Xu, dri-devel, lkml, Jose Abreu, Wolfram Sang,
	Mark Brown, Srinivas Kandagatla, Dave Long, Andy Green,
	Zhangfei Gao

On Tue, Aug 30, 2016 at 1:56 AM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> Hi John,
>
> Thank you for the patch.
>
> On Monday 29 Aug 2016 16:41:33 John Stultz wrote:
>> From: Archit Taneja <architt@codeaurora.org>
>>
>> This patch moves the adv7511 data structure to header file so that the
>> audio driver file could use it.
>
> Actually it doesn't, the data structure is already in the header file.

Heh.  Yea, it looks like most of that patch has fallen out, and
looking closer at it, the remainders aren't necessary, so I'm dropping
the whole thing.

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

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

* Re: [PATCH 4/4 v2] drm/bridge: adv7511: Initialize audio packet on adv7533
  2016-09-02  9:49     ` Archit Taneja
  (?)
@ 2016-09-06 22:58     ` John Stultz
  -1 siblings, 0 replies; 26+ messages in thread
From: John Stultz @ 2016-09-06 22:58 UTC (permalink / raw)
  To: Archit Taneja
  Cc: lkml, Andy Green, David Airlie, Laurent Pinchart, Wolfram Sang,
	Srinivas Kandagatla, Ville Syrjälä,
	Boris Brezillon, Andy Green, Dave Long, Guodong Xu, Zhangfei Gao,
	Mark Brown, Lars-Peter Clausen, Jose Abreu, dri-devel

On Fri, Sep 2, 2016 at 2:49 AM, Archit Taneja <architt@codeaurora.org> wrote:
> Hi,
>
>
> On 8/30/2016 5:11 AM, John Stultz wrote:
>>
>> From: Andy Green <andy.green@linaro.org>
>>
>> Set the initial audio packet settings to allow the audio
>> driver to work.
>>
>> Cc: David Airlie <airlied@linux.ie>
>> Cc: Archit Taneja <architt@codeaurora.org>
>> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>> Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
>> Cc: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>> Cc: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
>> Cc: Boris Brezillon <boris.brezillon@free-electrons.com>
>> Cc: Andy Green <andy@warmcat.com>
>> Cc: Dave Long <dave.long@linaro.org>
>> Cc: Guodong Xu <guodong.xu@linaro.org>
>> Cc: Zhangfei Gao <zhangfei.gao@linaro.org>
>> Cc: Mark Brown <broonie@kernel.org>
>> Cc: Lars-Peter Clausen <lars@metafoo.de>
>> Cc: Jose Abreu <joabreu@synopsys.com>
>> Cc: dri-devel@lists.freedesktop.org
>> Signed-off-by: Andy Green <andy.green@linaro.org>
>> [jstultz: Forward ported to mainline, changed to use register
>>   names rather then hex values, and removed config values set by
>>   audio driver.]
>> Signed-off-by: John Stultz <john.stultz@linaro.org>
>> ---
>>   drivers/gpu/drm/bridge/adv7511/adv7533.c | 22 ++++++++++++++++++++++
>>   1 file changed, 22 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7533.c
>> b/drivers/gpu/drm/bridge/adv7511/adv7533.c
>> index 6798ecf..cced7c9 100644
>> --- a/drivers/gpu/drm/bridge/adv7511/adv7533.c
>> +++ b/drivers/gpu/drm/bridge/adv7511/adv7533.c
>> @@ -104,6 +104,28 @@ void adv7533_dsi_power_on(struct adv7511 *adv)
>>         /* disable test mode */
>>         regmap_write(adv->regmap_cec, 0x55, 0x00);
>>
>> +       /* hide Audio infoframe updates */
>> +       regmap_update_bits(adv->regmap, ADV7511_REG_INFOFRAME_UPDATE,
>> +                               BIT(5), BIT(5));
>> +       /* enable N/CTS, enable Audio sample packets */
>> +       regmap_update_bits(adv->regmap, ADV7511_REG_PACKET_ENABLE1,
>> +                               BIT(5), BIT(5));
>> +       /* enable N/CTS */
>> +       regmap_update_bits(adv->regmap, ADV7511_REG_PACKET_ENABLE1,
>> +                               BIT(6), BIT(6));
>> +       /* not copyrighted */
>> +       regmap_update_bits(adv->regmap, ADV7511_REG_AUDIO_CFG1,
>> +                               BIT(5), BIT(5));
>> +       /* enable audio infoframes */
>> +       regmap_update_bits(adv->regmap, ADV7511_REG_PACKET_ENABLE1,
>> +                               BIT(3), BIT(3));
>> +       /* AV mute disable */
>> +       regmap_update_bits(adv->regmap, ADV7511_REG_GC(0),
>> +                               BIT(7) | BIT(6), BIT(7));
>> +       /* use Audio infoframe updated info */
>> +       regmap_update_bits(adv->regmap, ADV7511_REG_GC(1),
>> +                               BIT(5), 0);
>> +
>
>
> Wouldn't these writes be needed by ADV751x chips too? These seem to
> belong to the main ADV75xx regmap. These should probably be a separate func
> in adv7511_audio.c or adv7511_drv.c, and it should be called in
> adv7511_power_on, so that ADV751x chips can utilize this too.

Yea. It seems like folding this into the audio_startup() function is
the best thing here, so I've done so and it seems to be working well.
Will resend here shortly.

thanks
-john

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

* Re: [PATCH 0/4 v2] Audio support for adv7511 hdmi bridge
  2016-09-06 22:17   ` John Stultz
@ 2016-10-18 14:42       ` Laurent Pinchart
  0 siblings, 0 replies; 26+ messages in thread
From: Laurent Pinchart @ 2016-10-18 14:42 UTC (permalink / raw)
  To: John Stultz
  Cc: lkml, David Airlie, Archit Taneja, Wolfram Sang,
	Srinivas Kandagatla, Ville Syrjälä,
	Boris Brezillon, Andy Green, Dave Long, Guodong Xu, Zhangfei Gao,
	Mark Brown, Lars-Peter Clausen, Jose Abreu, dri-devel,
	kuninori.morimoto.gx

Hi John,

(CC'ing Morimoto-san)

On Tuesday 06 Sep 2016 15:17:58 John Stultz wrote:
> On Tue, Aug 30, 2016 at 2:23 AM, Laurent Pinchart wrote:
> > Hi John,
> > 
> > Thank you for the patches.
> 
> Thanks so much for the review! I'm reworking the patchset now and will
> be sending out an updated set soon!
> 
> >> This patchset, along with the i2s driver and dts changes allows
> >> HDMI audio to work on the HiKey board.
> > 
> > Where are the dts changes ?
> 
> Here's what I'm using to get it working:
> https://git.linaro.org/people/john.stultz/android-dev.git/commitdiff/1024cb4
> 85b6f00a7e355ce60425f04a584481148

Thank you.

We need to standardize DT bindings for HDMI sound output. Morimoto-san, could 
you have a look at the DT bindings proposal for HDMI sound output with the 
ADV7511, and comment on whether it matches the approach you've taken for HDMI 
sound output on R-Car Gen3 ?

> But again, that's dependent on the k3dma driver (queued), the hi6210
> i2s driver (still being reworked), and adv7511 audio (this patchset).

-- 
Regards,

Laurent Pinchart

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

* Re: [PATCH 0/4 v2] Audio support for adv7511 hdmi bridge
@ 2016-10-18 14:42       ` Laurent Pinchart
  0 siblings, 0 replies; 26+ messages in thread
From: Laurent Pinchart @ 2016-10-18 14:42 UTC (permalink / raw)
  To: John Stultz
  Cc: kuninori.morimoto.gx, Guodong Xu, dri-devel, lkml, Jose Abreu,
	Wolfram Sang, Mark Brown, Srinivas Kandagatla, Dave Long,
	Andy Green, Zhangfei Gao

Hi John,

(CC'ing Morimoto-san)

On Tuesday 06 Sep 2016 15:17:58 John Stultz wrote:
> On Tue, Aug 30, 2016 at 2:23 AM, Laurent Pinchart wrote:
> > Hi John,
> > 
> > Thank you for the patches.
> 
> Thanks so much for the review! I'm reworking the patchset now and will
> be sending out an updated set soon!
> 
> >> This patchset, along with the i2s driver and dts changes allows
> >> HDMI audio to work on the HiKey board.
> > 
> > Where are the dts changes ?
> 
> Here's what I'm using to get it working:
> https://git.linaro.org/people/john.stultz/android-dev.git/commitdiff/1024cb4
> 85b6f00a7e355ce60425f04a584481148

Thank you.

We need to standardize DT bindings for HDMI sound output. Morimoto-san, could 
you have a look at the DT bindings proposal for HDMI sound output with the 
ADV7511, and comment on whether it matches the approach you've taken for HDMI 
sound output on R-Car Gen3 ?

> But again, that's dependent on the k3dma driver (queued), the hi6210
> i2s driver (still being reworked), and adv7511 audio (this patchset).

-- 
Regards,

Laurent Pinchart

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

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

* Re: [PATCH 0/4 v2] Audio support for adv7511 hdmi bridge
  2016-10-18 14:42       ` Laurent Pinchart
  (?)
@ 2016-10-19  0:26       ` Kuninori Morimoto
  2016-11-01 21:41           ` John Stultz
  -1 siblings, 1 reply; 26+ messages in thread
From: Kuninori Morimoto @ 2016-10-19  0:26 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: John Stultz, lkml, David Airlie, Archit Taneja, Wolfram Sang,
	Srinivas Kandagatla, Ville Syrjälä,
	Boris Brezillon, Andy Green, Dave Long, Guodong Xu, Zhangfei Gao,
	Mark Brown, Lars-Peter Clausen, Jose Abreu, dri-devel


Hi Laurent, John,

> > Here's what I'm using to get it working:
> > https://git.linaro.org/people/john.stultz/android-dev.git/commitdiff/1024cb4
> > 85b6f00a7e355ce60425f04a584481148
> 
> Thank you.
> 
> We need to standardize DT bindings for HDMI sound output. Morimoto-san, could 
> you have a look at the DT bindings proposal for HDMI sound output with the 
> ADV7511, and comment on whether it matches the approach you've taken for HDMI 
> sound output on R-Car Gen3 ?

Above patch is using normal simple-card style for HDMI sound, but as Laurent
said we want to use same DT style for HDMI video and sound (= OF graph style).
Fortunately, I posted patch-set for OF graph support on sound card yesterday.
Can you check this ?
http://www.spinics.net/lists/devicetree/msg146131.html
The points are
 - sound can use OF graph style DT
 - sound-card DT is no longer needed
 - it needs type = "sound" property

patch-set [0/23] is this
http://www.spinics.net/lists/devicetree/msg146113.html

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

* Re: [PATCH 0/4 v2] Audio support for adv7511 hdmi bridge
  2016-10-19  0:26       ` Kuninori Morimoto
@ 2016-11-01 21:41           ` John Stultz
  0 siblings, 0 replies; 26+ messages in thread
From: John Stultz @ 2016-11-01 21:41 UTC (permalink / raw)
  To: Kuninori Morimoto
  Cc: Laurent Pinchart, lkml, David Airlie, Archit Taneja,
	Wolfram Sang, Srinivas Kandagatla, Ville Syrjälä,
	Boris Brezillon, Andy Green, Dave Long, Guodong Xu, Zhangfei Gao,
	Mark Brown, Lars-Peter Clausen, Jose Abreu, dri-devel

On Tue, Oct 18, 2016 at 5:26 PM, Kuninori Morimoto
<kuninori.morimoto.gx@renesas.com> wrote:
>
>> > Here's what I'm using to get it working:
>> > https://git.linaro.org/people/john.stultz/android-dev.git/commitdiff/1024cb4
>> > 85b6f00a7e355ce60425f04a584481148
>>
>> Thank you.
>>
>> We need to standardize DT bindings for HDMI sound output. Morimoto-san, could
>> you have a look at the DT bindings proposal for HDMI sound output with the
>> ADV7511, and comment on whether it matches the approach you've taken for HDMI
>> sound output on R-Car Gen3 ?
>
> Above patch is using normal simple-card style for HDMI sound, but as Laurent
> said we want to use same DT style for HDMI video and sound (= OF graph style).
> Fortunately, I posted patch-set for OF graph support on sound card yesterday.
> Can you check this ?
> http://www.spinics.net/lists/devicetree/msg146131.html
> The points are
>  - sound can use OF graph style DT
>  - sound-card DT is no longer needed
>  - it needs type = "sound" property
>
> patch-set [0/23] is this
> http://www.spinics.net/lists/devicetree/msg146113.html

Thanks for the pointers! If I understand this correctly, the OF graph
simple-card method would replace my current simple-card dts usage for
hikey?   Other then just having another out-of-tree patchset to track,
I don't have any objection to trying to use the OF graph simple-card
method in the hikey dts (though I suspect I'll have to pester you for
help when I give it a shot).

Laurent: So this seems to be mostly just a "which implementation do we
use" question for the hikey dts,  and it seems like the adv7511 audio
driver I'm submitting here has any ties to either the graph or normal
simple card method. Is that correct? Are there any changes to the
adv7511 audio support patch you'd like to see or should I just
resubmit it?

thanks
-john

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

* Re: [PATCH 0/4 v2] Audio support for adv7511 hdmi bridge
@ 2016-11-01 21:41           ` John Stultz
  0 siblings, 0 replies; 26+ messages in thread
From: John Stultz @ 2016-11-01 21:41 UTC (permalink / raw)
  To: Kuninori Morimoto
  Cc: Guodong Xu, dri-devel, lkml, Jose Abreu, Wolfram Sang,
	Mark Brown, Srinivas Kandagatla, Laurent Pinchart, Andy Green,
	Zhangfei Gao, Dave Long

On Tue, Oct 18, 2016 at 5:26 PM, Kuninori Morimoto
<kuninori.morimoto.gx@renesas.com> wrote:
>
>> > Here's what I'm using to get it working:
>> > https://git.linaro.org/people/john.stultz/android-dev.git/commitdiff/1024cb4
>> > 85b6f00a7e355ce60425f04a584481148
>>
>> Thank you.
>>
>> We need to standardize DT bindings for HDMI sound output. Morimoto-san, could
>> you have a look at the DT bindings proposal for HDMI sound output with the
>> ADV7511, and comment on whether it matches the approach you've taken for HDMI
>> sound output on R-Car Gen3 ?
>
> Above patch is using normal simple-card style for HDMI sound, but as Laurent
> said we want to use same DT style for HDMI video and sound (= OF graph style).
> Fortunately, I posted patch-set for OF graph support on sound card yesterday.
> Can you check this ?
> http://www.spinics.net/lists/devicetree/msg146131.html
> The points are
>  - sound can use OF graph style DT
>  - sound-card DT is no longer needed
>  - it needs type = "sound" property
>
> patch-set [0/23] is this
> http://www.spinics.net/lists/devicetree/msg146113.html

Thanks for the pointers! If I understand this correctly, the OF graph
simple-card method would replace my current simple-card dts usage for
hikey?   Other then just having another out-of-tree patchset to track,
I don't have any objection to trying to use the OF graph simple-card
method in the hikey dts (though I suspect I'll have to pester you for
help when I give it a shot).

Laurent: So this seems to be mostly just a "which implementation do we
use" question for the hikey dts,  and it seems like the adv7511 audio
driver I'm submitting here has any ties to either the graph or normal
simple card method. Is that correct? Are there any changes to the
adv7511 audio support patch you'd like to see or should I just
resubmit it?

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

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

* Re: [PATCH 0/4 v2] Audio support for adv7511 hdmi bridge
  2016-11-01 21:41           ` John Stultz
  (?)
@ 2016-11-02  1:43           ` Kuninori Morimoto
  -1 siblings, 0 replies; 26+ messages in thread
From: Kuninori Morimoto @ 2016-11-02  1:43 UTC (permalink / raw)
  To: John Stultz
  Cc: Laurent Pinchart, lkml, David Airlie, Archit Taneja,
	Wolfram Sang, Srinivas Kandagatla, Ville Syrjälä,
	Boris Brezillon, Andy Green, Dave Long, Guodong Xu, Zhangfei Gao,
	Mark Brown, Lars-Peter Clausen, Jose Abreu, dri-devel


Hi John

> > Above patch is using normal simple-card style for HDMI sound, but as Laurent
> > said we want to use same DT style for HDMI video and sound (= OF graph style).
> > Fortunately, I posted patch-set for OF graph support on sound card yesterday.
> > Can you check this ?
> > http://www.spinics.net/lists/devicetree/msg146131.html
> > The points are
> >  - sound can use OF graph style DT
> >  - sound-card DT is no longer needed
> >  - it needs type = "sound" property
> >
> > patch-set [0/23] is this
> > http://www.spinics.net/lists/devicetree/msg146113.html
> 
> Thanks for the pointers! If I understand this correctly, the OF graph
> simple-card method would replace my current simple-card dts usage for
> hikey?   Other then just having another out-of-tree patchset to track,
> I don't have any objection to trying to use the OF graph simple-card
> method in the hikey dts (though I suspect I'll have to pester you for
> help when I give it a shot).

I posted v2 patchset OF graph simple-card which doesn't have "type" property.
Anyway, if you use OF graph simple-card, your "CPU" side driver need to
call asoc_simple_card_try_to_probe_graph_card() to probing it.

And this is not 100% mandatory, but your CPU and/or Codec driver
want to adjust OF graph style for parsing. The position is like this

simple-card :: dai-link <-> OF graph simple-card :: port/endpoint

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

end of thread, other threads:[~2016-11-02  1:43 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-29 23:41 [PATCH 0/4 v2] Audio support for adv7511 hdmi bridge John Stultz
2016-08-29 23:41 ` John Stultz
2016-08-29 23:41 ` [PATCH 1/4 v2] drm/bridge: adv7511: Move the common data structures to header file John Stultz
2016-08-29 23:41   ` John Stultz
2016-08-30  8:56   ` Laurent Pinchart
2016-08-30  8:56     ` Laurent Pinchart
2016-09-06 22:49     ` John Stultz
2016-09-06 22:49       ` John Stultz
2016-08-29 23:41 ` [PATCH 2/4 v2] drm/bridge: adv7511: Add Audio support John Stultz
2016-08-30  9:21   ` Laurent Pinchart
2016-08-30  9:21     ` Laurent Pinchart
2016-08-29 23:41 ` [PATCH 3/4 v2] drm/bridge: adv7511: Enable the audio data and clock pads on adv7533 John Stultz
2016-08-29 23:41   ` John Stultz
2016-08-29 23:41 ` [PATCH 4/4 v2] drm/bridge: adv7511: Initialize audio packet " John Stultz
2016-09-02  9:49   ` Archit Taneja
2016-09-02  9:49     ` Archit Taneja
2016-09-06 22:58     ` John Stultz
2016-08-30  9:23 ` [PATCH 0/4 v2] Audio support for adv7511 hdmi bridge Laurent Pinchart
2016-08-30  9:23   ` Laurent Pinchart
2016-09-06 22:17   ` John Stultz
2016-10-18 14:42     ` Laurent Pinchart
2016-10-18 14:42       ` Laurent Pinchart
2016-10-19  0:26       ` Kuninori Morimoto
2016-11-01 21:41         ` John Stultz
2016-11-01 21:41           ` John Stultz
2016-11-02  1:43           ` Kuninori Morimoto

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.