All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2 v3] Audio support for adv7511 hdmi bridge
@ 2016-09-06 23:22 John Stultz
  2016-09-06 23:22   ` John Stultz
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: John Stultz @ 2016-09-06 23:22 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 v3:
* Dropped unnecessary first patch
* Made a few clenaups suggested by Laurent
* Made audio a config time option, as suggested by Laurent
* Folded in Andy Green's audio packet initialization patch

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

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         |   9 ++
 drivers/gpu/drm/bridge/adv7511/Makefile        |   1 +
 drivers/gpu/drm/bridge/adv7511/adv7511.h       |  16 ++
 drivers/gpu/drm/bridge/adv7511/adv7511_audio.c | 213 +++++++++++++++++++++++++
 drivers/gpu/drm/bridge/adv7511/adv7511_drv.c   |   4 +
 drivers/gpu/drm/bridge/adv7511/adv7533.c       |   1 +
 6 files changed, 244 insertions(+)
 create mode 100644 drivers/gpu/drm/bridge/adv7511/adv7511_audio.c

-- 
1.9.1

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

* [PATCH 1/2 v3] drm/bridge: adv7511: Add Audio support.
  2016-09-06 23:22 [PATCH 0/2 v3] Audio support for adv7511 hdmi bridge John Stultz
@ 2016-09-06 23:22   ` John Stultz
  2016-09-06 23:22   ` John Stultz
  2016-09-12 21:58 ` [PATCH 0/2 v3] Audio support for adv7511 hdmi bridge John Stultz
  2 siblings, 0 replies; 10+ messages in thread
From: John Stultz @ 2016-09-06 23:22 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. And also
folded in some audio packet initialization done by Andy Green
<andy.green@linaro.org>. 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>
---
v3:
* Allowed audio support to be configured in or out, as suggested by Laurent
* Minor cleanups suggested by Laurent
* Folded in Andy's audio packet initialization patch, as suggested by Archit

 drivers/gpu/drm/bridge/adv7511/Kconfig         |   9 ++
 drivers/gpu/drm/bridge/adv7511/Makefile        |   1 +
 drivers/gpu/drm/bridge/adv7511/adv7511.h       |  16 ++
 drivers/gpu/drm/bridge/adv7511/adv7511_audio.c | 213 +++++++++++++++++++++++++
 drivers/gpu/drm/bridge/adv7511/adv7511_drv.c   |   4 +
 5 files changed, 243 insertions(+)
 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..b303ad1 100644
--- a/drivers/gpu/drm/bridge/adv7511/Kconfig
+++ b/drivers/gpu/drm/bridge/adv7511/Kconfig
@@ -3,9 +3,18 @@ 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.
 
+config DRM_I2C_ADV7511_AUDIO
+	tristate "ADV7511 HDMI Audio driver"
+	depends on DRM_I2C_ADV7511 && SND_SOC
+	select SND_SOC_HDMI_CODEC
+	help
+	  Support the ADV7511 HDMI Audio interface. This is used in
+	  conjunction with the AV7511  HDMI driver.
+
 config DRM_I2C_ADV7533
 	bool "ADV7533 encoder"
 	depends on DRM_I2C_ADV7511
diff --git a/drivers/gpu/drm/bridge/adv7511/Makefile b/drivers/gpu/drm/bridge/adv7511/Makefile
index 9019327..5ba6755 100644
--- a/drivers/gpu/drm/bridge/adv7511/Makefile
+++ b/drivers/gpu/drm/bridge/adv7511/Makefile
@@ -1,3 +1,4 @@
 adv7511-y := adv7511_drv.o
+adv7511-$(CONFIG_DRM_I2C_ADV7511_AUDIO) += 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 161c923..992d76c 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511.h
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511.h
@@ -309,6 +309,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];
@@ -334,6 +336,7 @@ struct adv7511 {
 	bool use_timing_gen;
 
 	enum adv7511_type type;
+	struct platform_device *audio_pdev;
 };
 
 #ifdef CONFIG_DRM_I2C_ADV7533
@@ -389,4 +392,17 @@ static inline int adv7533_parse_dt(struct device_node *np, struct adv7511 *adv)
 }
 #endif
 
+#ifdef CONFIG_DRM_I2C_ADV7511_AUDIO
+int adv7511_audio_init(struct device *dev, struct adv7511 *adv7511);
+void adv7511_audio_exit(struct adv7511 *adv7511);
+#else /*CONFIG_DRM_I2C_ADV7511_AUDIO */
+static inline int adv7511_audio_init(struct device *dev, struct adv7511 *adv7511)
+{
+	return 0;
+}
+static inline void adv7511_audio_exit(struct adv7511 *adv7511)
+{
+}
+#endif /* CONFIG_DRM_I2C_ADV7511_AUDIO */
+
 #endif /* __DRM_I2C_ADV7511_H__ */
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..5ce29a5
--- /dev/null
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511_audio.c
@@ -0,0 +1,213 @@
+/*
+ * 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;
+	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);
+
+	/* hide Audio infoframe updates */
+	regmap_update_bits(adv7511->regmap, ADV7511_REG_INFOFRAME_UPDATE,
+				BIT(5), BIT(5));
+	/* enable N/CTS, enable Audio sample packets */
+	regmap_update_bits(adv7511->regmap, ADV7511_REG_PACKET_ENABLE1,
+				BIT(5), BIT(5));
+	/* enable N/CTS */
+	regmap_update_bits(adv7511->regmap, ADV7511_REG_PACKET_ENABLE1,
+				BIT(6), BIT(6));
+	/* not copyrighted */
+	regmap_update_bits(adv7511->regmap, ADV7511_REG_AUDIO_CFG1,
+				BIT(5), BIT(5));
+	/* enable audio infoframes */
+	regmap_update_bits(adv7511->regmap, ADV7511_REG_PACKET_ENABLE1,
+				BIT(3), BIT(3));
+	/* AV mute disable */
+	regmap_update_bits(adv7511->regmap, ADV7511_REG_GC(0),
+				BIT(7) | BIT(6), BIT(7));
+	/* use Audio infoframe updated info */
+	regmap_update_bits(adv7511->regmap, ADV7511_REG_GC(1),
+				BIT(5), 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)
+{
+	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 adv7511 *adv7511)
+{
+	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 ec8fb2e..d0b0c6a 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
@@ -1037,6 +1037,8 @@ static int adv7511_probe(struct i2c_client *i2c, const struct i2c_device_id *id)
 		goto err_unregister_cec;
 	}
 
+	adv7511_audio_init(dev, adv7511);
+
 	return 0;
 
 err_unregister_cec:
@@ -1058,6 +1060,8 @@ static int adv7511_remove(struct i2c_client *i2c)
 
 	drm_bridge_remove(&adv7511->bridge);
 
+	adv7511_audio_exit(adv7511);
+
 	i2c_unregister_device(adv7511->i2c_edid);
 
 	kfree(adv7511->edid);
-- 
1.9.1

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

* [PATCH 1/2 v3] drm/bridge: adv7511: Add Audio support.
@ 2016-09-06 23:22   ` John Stultz
  0 siblings, 0 replies; 10+ messages in thread
From: John Stultz @ 2016-09-06 23:22 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 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. And also
folded in some audio packet initialization done by Andy Green
<andy.green@linaro.org>. 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>
---
v3:
* Allowed audio support to be configured in or out, as suggested by Laurent
* Minor cleanups suggested by Laurent
* Folded in Andy's audio packet initialization patch, as suggested by Archit

 drivers/gpu/drm/bridge/adv7511/Kconfig         |   9 ++
 drivers/gpu/drm/bridge/adv7511/Makefile        |   1 +
 drivers/gpu/drm/bridge/adv7511/adv7511.h       |  16 ++
 drivers/gpu/drm/bridge/adv7511/adv7511_audio.c | 213 +++++++++++++++++++++++++
 drivers/gpu/drm/bridge/adv7511/adv7511_drv.c   |   4 +
 5 files changed, 243 insertions(+)
 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..b303ad1 100644
--- a/drivers/gpu/drm/bridge/adv7511/Kconfig
+++ b/drivers/gpu/drm/bridge/adv7511/Kconfig
@@ -3,9 +3,18 @@ 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.
 
+config DRM_I2C_ADV7511_AUDIO
+	tristate "ADV7511 HDMI Audio driver"
+	depends on DRM_I2C_ADV7511 && SND_SOC
+	select SND_SOC_HDMI_CODEC
+	help
+	  Support the ADV7511 HDMI Audio interface. This is used in
+	  conjunction with the AV7511  HDMI driver.
+
 config DRM_I2C_ADV7533
 	bool "ADV7533 encoder"
 	depends on DRM_I2C_ADV7511
diff --git a/drivers/gpu/drm/bridge/adv7511/Makefile b/drivers/gpu/drm/bridge/adv7511/Makefile
index 9019327..5ba6755 100644
--- a/drivers/gpu/drm/bridge/adv7511/Makefile
+++ b/drivers/gpu/drm/bridge/adv7511/Makefile
@@ -1,3 +1,4 @@
 adv7511-y := adv7511_drv.o
+adv7511-$(CONFIG_DRM_I2C_ADV7511_AUDIO) += 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 161c923..992d76c 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511.h
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511.h
@@ -309,6 +309,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];
@@ -334,6 +336,7 @@ struct adv7511 {
 	bool use_timing_gen;
 
 	enum adv7511_type type;
+	struct platform_device *audio_pdev;
 };
 
 #ifdef CONFIG_DRM_I2C_ADV7533
@@ -389,4 +392,17 @@ static inline int adv7533_parse_dt(struct device_node *np, struct adv7511 *adv)
 }
 #endif
 
+#ifdef CONFIG_DRM_I2C_ADV7511_AUDIO
+int adv7511_audio_init(struct device *dev, struct adv7511 *adv7511);
+void adv7511_audio_exit(struct adv7511 *adv7511);
+#else /*CONFIG_DRM_I2C_ADV7511_AUDIO */
+static inline int adv7511_audio_init(struct device *dev, struct adv7511 *adv7511)
+{
+	return 0;
+}
+static inline void adv7511_audio_exit(struct adv7511 *adv7511)
+{
+}
+#endif /* CONFIG_DRM_I2C_ADV7511_AUDIO */
+
 #endif /* __DRM_I2C_ADV7511_H__ */
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..5ce29a5
--- /dev/null
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511_audio.c
@@ -0,0 +1,213 @@
+/*
+ * 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;
+	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);
+
+	/* hide Audio infoframe updates */
+	regmap_update_bits(adv7511->regmap, ADV7511_REG_INFOFRAME_UPDATE,
+				BIT(5), BIT(5));
+	/* enable N/CTS, enable Audio sample packets */
+	regmap_update_bits(adv7511->regmap, ADV7511_REG_PACKET_ENABLE1,
+				BIT(5), BIT(5));
+	/* enable N/CTS */
+	regmap_update_bits(adv7511->regmap, ADV7511_REG_PACKET_ENABLE1,
+				BIT(6), BIT(6));
+	/* not copyrighted */
+	regmap_update_bits(adv7511->regmap, ADV7511_REG_AUDIO_CFG1,
+				BIT(5), BIT(5));
+	/* enable audio infoframes */
+	regmap_update_bits(adv7511->regmap, ADV7511_REG_PACKET_ENABLE1,
+				BIT(3), BIT(3));
+	/* AV mute disable */
+	regmap_update_bits(adv7511->regmap, ADV7511_REG_GC(0),
+				BIT(7) | BIT(6), BIT(7));
+	/* use Audio infoframe updated info */
+	regmap_update_bits(adv7511->regmap, ADV7511_REG_GC(1),
+				BIT(5), 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)
+{
+	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 adv7511 *adv7511)
+{
+	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 ec8fb2e..d0b0c6a 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
@@ -1037,6 +1037,8 @@ static int adv7511_probe(struct i2c_client *i2c, const struct i2c_device_id *id)
 		goto err_unregister_cec;
 	}
 
+	adv7511_audio_init(dev, adv7511);
+
 	return 0;
 
 err_unregister_cec:
@@ -1058,6 +1060,8 @@ static int adv7511_remove(struct i2c_client *i2c)
 
 	drm_bridge_remove(&adv7511->bridge);
 
+	adv7511_audio_exit(adv7511);
+
 	i2c_unregister_device(adv7511->i2c_edid);
 
 	kfree(adv7511->edid);
-- 
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] 10+ messages in thread

* [PATCH 2/2 v3] drm/bridge: adv7511: Enable the audio data and clock pads on adv7533
  2016-09-06 23:22 [PATCH 0/2 v3] Audio support for adv7511 hdmi bridge John Stultz
@ 2016-09-06 23:22   ` John Stultz
  2016-09-06 23:22   ` John Stultz
  2016-09-12 21:58 ` [PATCH 0/2 v3] Audio support for adv7511 hdmi bridge John Stultz
  2 siblings, 0 replies; 10+ messages in thread
From: John Stultz @ 2016-09-06 23:22 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] 10+ messages in thread

* [PATCH 2/2 v3] drm/bridge: adv7511: Enable the audio data and clock pads on adv7533
@ 2016-09-06 23:22   ` John Stultz
  0 siblings, 0 replies; 10+ messages in thread
From: John Stultz @ 2016-09-06 23:22 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] 10+ messages in thread

* Re: [PATCH 0/2 v3] Audio support for adv7511 hdmi bridge
  2016-09-06 23:22 [PATCH 0/2 v3] Audio support for adv7511 hdmi bridge John Stultz
  2016-09-06 23:22   ` John Stultz
  2016-09-06 23:22   ` John Stultz
@ 2016-09-12 21:58 ` John Stultz
  2016-09-12 22:27   ` Mark Brown
  2 siblings, 1 reply; 10+ messages in thread
From: John Stultz @ 2016-09-12 21:58 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

On Tue, Sep 6, 2016 at 4:22 PM, John Stultz <john.stultz@linaro.org> 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.
>
> I'd really appreciate any thoughts or feedback.
>
> New in v3:
> * Dropped unnecessary first patch
> * Made a few clenaups suggested by Laurent
> * Made audio a config time option, as suggested by Laurent
> * Folded in Andy Green's audio packet initialization patch

Ping? Just wanted to see if there was any other feedback on this?

thanks
-john

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

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

[-- Attachment #1: Type: text/plain, Size: 657 bytes --]

On Mon, Sep 12, 2016 at 02:58:44PM -0700, John Stultz wrote:

> Ping? Just wanted to see if there was any other feedback on this?

Please don't send content free pings and please allow a reasonable time
for review.  People get busy, go on holiday, attend conferences and so 
on so unless there is some reason for urgency (like critical bug fixes)
please allow at least a couple of weeks for review.  If there have been
review comments then people may be waiting for those to be addressed.
Sending content free pings just adds to the mail volume (if they are
seen at all) and if something has gone wrong you'll have to resend the
patches anyway.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

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

* Re: [PATCH 1/2 v3] drm/bridge: adv7511: Add Audio support.
  2016-09-06 23:22   ` John Stultz
  (?)
@ 2016-09-16 10:51   ` Lars-Peter Clausen
  2016-09-16 23:08       ` John Stultz
  -1 siblings, 1 reply; 10+ messages in thread
From: Lars-Peter Clausen @ 2016-09-16 10:51 UTC (permalink / raw)
  To: John Stultz, lkml
  Cc: 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, Jose Abreu, dri-devel

On 09/07/2016 01:22 AM, 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. And also
> folded in some audio packet initialization done by Andy Green
> <andy.green@linaro.org>. 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>

I'm not to fond of the hdmi-codec stuff, but within its context this looks
ok. Thanks for getting this upstream ready. Just one tiny bit regarding the
Kconfig entry.

If that is fixed feel free to add on the next revision:

Acked-by: Lars-Peter Clausen <lars@metafoo.de>

> ---
> v3:
> * Allowed audio support to be configured in or out, as suggested by Laurent
> * Minor cleanups suggested by Laurent
> * Folded in Andy's audio packet initialization patch, as suggested by Archit
> 
>  drivers/gpu/drm/bridge/adv7511/Kconfig         |   9 ++
>  drivers/gpu/drm/bridge/adv7511/Makefile        |   1 +
>  drivers/gpu/drm/bridge/adv7511/adv7511.h       |  16 ++
>  drivers/gpu/drm/bridge/adv7511/adv7511_audio.c | 213 +++++++++++++++++++++++++
>  drivers/gpu/drm/bridge/adv7511/adv7511_drv.c   |   4 +
>  5 files changed, 243 insertions(+)
>  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..b303ad1 100644
> --- a/drivers/gpu/drm/bridge/adv7511/Kconfig
> +++ b/drivers/gpu/drm/bridge/adv7511/Kconfig
> @@ -3,9 +3,18 @@ config DRM_I2C_ADV7511
>  	depends on OF
>  	select DRM_KMS_HELPER
>  	select REGMAP_I2C
> +	select SND_SOC_HDMI_CODEC if SND_SOC

This seems to be redundant considering the select done by DRM_I2C_ADV7511_AUDIO.

>  	help
>  	  Support for the Analog Device ADV7511(W) and ADV7513 HDMI encoders.
>  
> +config DRM_I2C_ADV7511_AUDIO
> +	tristate "ADV7511 HDMI Audio driver"

This should be bool. It either gets built into the ADV7511 driver (which
itself could either be a module or built-in) or not. But setting this option
itself to module wont work.

> +	depends on DRM_I2C_ADV7511 && SND_SOC
> +	select SND_SOC_HDMI_CODEC
> +	help
> +	  Support the ADV7511 HDMI Audio interface. This is used in
> +	  conjunction with the AV7511  HDMI driver.
> +

[...]
> +static void audio_shutdown(struct device *dev, void *data)
> +{
> +}

Unrelated to this patch, but it looks like the shutdown callback should
maybe be made optional.

> +static const struct hdmi_codec_ops adv7511_codec_ops = {
> +	.hw_params	= adv7511_hdmi_hw_params,
> +	.audio_shutdown = audio_shutdown,
> +	.audio_startup	= audio_startup,
> +};

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

* Re: [PATCH 1/2 v3] drm/bridge: adv7511: Add Audio support.
  2016-09-16 10:51   ` Lars-Peter Clausen
@ 2016-09-16 23:08       ` John Stultz
  0 siblings, 0 replies; 10+ messages in thread
From: John Stultz @ 2016-09-16 23:08 UTC (permalink / raw)
  To: Lars-Peter Clausen
  Cc: lkml, 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, Jose Abreu, dri-devel

On Fri, Sep 16, 2016 at 3:51 AM, Lars-Peter Clausen <lars@metafoo.de> wrote:
> On 09/07/2016 01:22 AM, 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. And also
>> folded in some audio packet initialization done by Andy Green
>> <andy.green@linaro.org>. 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>
>
> I'm not to fond of the hdmi-codec stuff, but within its context this looks
> ok. Thanks for getting this upstream ready. Just one tiny bit regarding the
> Kconfig entry.
>
> If that is fixed feel free to add on the next revision:
>
> Acked-by: Lars-Peter Clausen <lars@metafoo.de>
>
>> ---
>> v3:
>> * Allowed audio support to be configured in or out, as suggested by Laurent
>> * Minor cleanups suggested by Laurent
>> * Folded in Andy's audio packet initialization patch, as suggested by Archit
>>
>>  drivers/gpu/drm/bridge/adv7511/Kconfig         |   9 ++
>>  drivers/gpu/drm/bridge/adv7511/Makefile        |   1 +
>>  drivers/gpu/drm/bridge/adv7511/adv7511.h       |  16 ++
>>  drivers/gpu/drm/bridge/adv7511/adv7511_audio.c | 213 +++++++++++++++++++++++++
>>  drivers/gpu/drm/bridge/adv7511/adv7511_drv.c   |   4 +
>>  5 files changed, 243 insertions(+)
>>  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..b303ad1 100644
>> --- a/drivers/gpu/drm/bridge/adv7511/Kconfig
>> +++ b/drivers/gpu/drm/bridge/adv7511/Kconfig
>> @@ -3,9 +3,18 @@ config DRM_I2C_ADV7511
>>       depends on OF
>>       select DRM_KMS_HELPER
>>       select REGMAP_I2C
>> +     select SND_SOC_HDMI_CODEC if SND_SOC
>
> This seems to be redundant considering the select done by DRM_I2C_ADV7511_AUDIO.
>
>>       help
>>         Support for the Analog Device ADV7511(W) and ADV7513 HDMI encoders.
>>
>> +config DRM_I2C_ADV7511_AUDIO
>> +     tristate "ADV7511 HDMI Audio driver"
>
> This should be bool. It either gets built into the ADV7511 driver (which
> itself could either be a module or built-in) or not. But setting this option
> itself to module wont work.

Ah! Thanks for the catch here!

I really appreciate the feedback! I'll integrate those changes and
respin the patch here shortly!

thanks
-john

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

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

On Fri, Sep 16, 2016 at 3:51 AM, Lars-Peter Clausen <lars@metafoo.de> wrote:
> On 09/07/2016 01:22 AM, 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. And also
>> folded in some audio packet initialization done by Andy Green
>> <andy.green@linaro.org>. 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>
>
> I'm not to fond of the hdmi-codec stuff, but within its context this looks
> ok. Thanks for getting this upstream ready. Just one tiny bit regarding the
> Kconfig entry.
>
> If that is fixed feel free to add on the next revision:
>
> Acked-by: Lars-Peter Clausen <lars@metafoo.de>
>
>> ---
>> v3:
>> * Allowed audio support to be configured in or out, as suggested by Laurent
>> * Minor cleanups suggested by Laurent
>> * Folded in Andy's audio packet initialization patch, as suggested by Archit
>>
>>  drivers/gpu/drm/bridge/adv7511/Kconfig         |   9 ++
>>  drivers/gpu/drm/bridge/adv7511/Makefile        |   1 +
>>  drivers/gpu/drm/bridge/adv7511/adv7511.h       |  16 ++
>>  drivers/gpu/drm/bridge/adv7511/adv7511_audio.c | 213 +++++++++++++++++++++++++
>>  drivers/gpu/drm/bridge/adv7511/adv7511_drv.c   |   4 +
>>  5 files changed, 243 insertions(+)
>>  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..b303ad1 100644
>> --- a/drivers/gpu/drm/bridge/adv7511/Kconfig
>> +++ b/drivers/gpu/drm/bridge/adv7511/Kconfig
>> @@ -3,9 +3,18 @@ config DRM_I2C_ADV7511
>>       depends on OF
>>       select DRM_KMS_HELPER
>>       select REGMAP_I2C
>> +     select SND_SOC_HDMI_CODEC if SND_SOC
>
> This seems to be redundant considering the select done by DRM_I2C_ADV7511_AUDIO.
>
>>       help
>>         Support for the Analog Device ADV7511(W) and ADV7513 HDMI encoders.
>>
>> +config DRM_I2C_ADV7511_AUDIO
>> +     tristate "ADV7511 HDMI Audio driver"
>
> This should be bool. It either gets built into the ADV7511 driver (which
> itself could either be a module or built-in) or not. But setting this option
> itself to module wont work.

Ah! Thanks for the catch here!

I really appreciate the feedback! I'll integrate those changes and
respin the patch here shortly!

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

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

end of thread, other threads:[~2016-09-16 23:08 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-06 23:22 [PATCH 0/2 v3] Audio support for adv7511 hdmi bridge John Stultz
2016-09-06 23:22 ` [PATCH 1/2 v3] drm/bridge: adv7511: Add Audio support John Stultz
2016-09-06 23:22   ` John Stultz
2016-09-16 10:51   ` Lars-Peter Clausen
2016-09-16 23:08     ` John Stultz
2016-09-16 23:08       ` John Stultz
2016-09-06 23:22 ` [PATCH 2/2 v3] drm/bridge: adv7511: Enable the audio data and clock pads on adv7533 John Stultz
2016-09-06 23:22   ` John Stultz
2016-09-12 21:58 ` [PATCH 0/2 v3] Audio support for adv7511 hdmi bridge John Stultz
2016-09-12 22:27   ` Mark Brown

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.