Audio support for adv7511 hdmi bridge

From: Archit Taneja <[email protected]>

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

Cc David Airlie <[email protected]>

Cc Archit Taneja <[email protected]>

Cc Laurent Pinchart <[email protected]>

Cc Wolfram Sang <[email protected]>

Cc Srinivas Kandagatla <[email protected]>

Cc "Ville Syrjälä" <[email protected]>

Cc Boris Brezillon <[email protected]>

Cc Andy Green <[email protected]>

Cc Dave Long <[email protected]>

Cc Guodong Xu <[email protected]>

Cc Zhangfei Gao <[email protected]>

Cc Mark Brown <[email protected]>

Cc Lars-Peter Clausen <[email protected]>

Cc Jose Abreu <[email protected]>

Cc [email protected]

Signed-off-by Srinivas Kandagatla <[email protected]>

Signed-off-by John Stultz <[email protected]>

---
 drivers/gpu/drm/bridge/adv7511/adv7511.h     | 8 ++++++++
 drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 4 ++--
 2 files changed, 10 insertions(+), 2 deletions(-)

(quoted diff) +0 -0

b/drivers/gpu/drm/bridge/adv7511/adv7511.h index 161c923..c7002a0 100644

drivers/gpu/drm/bridge/adv7511/adv7511.h +0 -0

 #define ADV7511_REG_CHIP_REVISION		0x00
 #define ADV7511_REG_N0				0x01
 #define ADV7511_REG_N1				0x02

(quoted diff) +0 -0

b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c index ec8fb2e..f8eb7f8 100644

drivers/gpu/drm/bridge/adv7511/adv7511_drv.c +1 -1

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

drivers/gpu/drm/bridge/adv7511/adv7511_drv.c +1 -1

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

Hi John,

Thank you for the patch.

Quoted 4 lines · On Monday 29 Aug 2016 16:41:33 John Stultz wrote:

From: Archit Taneja <[email protected]>

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.

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

This patch was originally from [1] by Lars-Peter Clausen <[email protected]> and was adapted by Archit Taneja <[email protected]> and Srinivas Kandagatla <[email protected]>.

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 <[email protected]>

Cc Archit Taneja <[email protected]>

Cc Laurent Pinchart <[email protected]>

Cc Wolfram Sang <[email protected]>

Cc Srinivas Kandagatla <[email protected]>

Cc "Ville Syrjälä" <[email protected]>

Cc Boris Brezillon <[email protected]>

Cc Andy Green <[email protected]>

Cc Dave Long <[email protected]>

Cc Guodong Xu <[email protected]>

Cc Zhangfei Gao <[email protected]>

Cc Mark Brown <[email protected]>

Cc Lars-Peter Clausen <[email protected]>

Cc Jose Abreu <[email protected]>

Cc [email protected]

Signed-off-by John Stultz <[email protected]>

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

(quoted diff) +0 -0

b/drivers/gpu/drm/bridge/adv7511/Kconfig index d2b0499..e3975e9 100644

drivers/gpu/drm/bridge/adv7511/Makefile +0 -0

 adv7511-$(CONFIG_DRM_I2C_ADV7533) += adv7533.o
 obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511.o

(quoted diff) +0 -0

b/drivers/gpu/drm/bridge/adv7511/adv7511.h index c7002a0..2e4d340 100644

drivers/gpu/drm/bridge/adv7511/adv7511.h +5 -0

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

(quoted diff) +0 -0

b/drivers/gpu/drm/bridge/adv7511/adv7511_audio.c new file mode 100644 index 0000000..0e0ea6b

drivers/gpu/drm/bridge/adv7511/adv7511_audio.c new file +114 -0

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

drivers/gpu/drm/bridge/adv7511/adv7511_audio.c new file +30 -0

+	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)
+{

drivers/gpu/drm/bridge/adv7511/adv7511_audio.c new file +7 -0

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

drivers/gpu/drm/bridge/adv7511/adv7511_audio.c new file +6 -0

+
+	if (adv7511->audio_pdev) {
+		platform_device_unregister(adv7511->audio_pdev);
+		adv7511->audio_pdev = NULL;
+	}
+}

(quoted diff) +0 -0

b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c index f8eb7f8..3756994 100644

drivers/gpu/drm/bridge/adv7511/adv7511_drv.c +4 -0

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

From: Andy Green <[email protected]>

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

Cc David Airlie <[email protected]>

Cc Archit Taneja <[email protected]>

Cc Laurent Pinchart <[email protected]>

Cc Wolfram Sang <[email protected]>

Cc Srinivas Kandagatla <[email protected]>

Cc "Ville Syrjälä" <[email protected]>

Cc Boris Brezillon <[email protected]>

Cc Andy Green <[email protected]>

Cc Dave Long <[email protected]>

Cc Guodong Xu <[email protected]>

Cc Zhangfei Gao <[email protected]>

Cc Mark Brown <[email protected]>

Cc Lars-Peter Clausen <[email protected]>

Cc Jose Abreu <[email protected]>

Cc [email protected]

Signed-off-by Andy Green <[email protected]>

[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 <[email protected]>

---
  drivers/gpu/drm/bridge/adv7511/adv7533.c | 22 ++++++++++++++++++++++
  1 file changed, 22 insertions(+)

drivers/gpu/drm/bridge/adv7511/adv7533.c +12 -0

index 6798ecf..cced7c9 100644
@@ -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));

drivers/gpu/drm/bridge/adv7511/adv7533.c +0 -0

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

Hi,

Quoted 59 lines · John Stultz

From: Andy Green <[email protected]>

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

Cc David Airlie <[email protected]>

Cc Archit Taneja <[email protected]>

Cc Laurent Pinchart <[email protected]>

Cc Wolfram Sang <[email protected]>

Cc Srinivas Kandagatla <[email protected]>

Cc "Ville Syrjälä" <[email protected]>

Cc Boris Brezillon <[email protected]>

Cc Andy Green <[email protected]>

Cc Dave Long <[email protected]>

Cc Guodong Xu <[email protected]>

Cc Zhangfei Gao <[email protected]>

Cc Mark Brown <[email protected]>

Cc Lars-Peter Clausen <[email protected]>

Cc Jose Abreu <[email protected]>

Cc [email protected]

Signed-off-by Andy Green <[email protected]>

[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 <[email protected]>

---
  drivers/gpu/drm/bridge/adv7511/adv7533.c | 22 ++++++++++++++++++++++
  1 file changed, 22 insertions(+)

drivers/gpu/drm/bridge/adv7511/adv7533.c +20 -0

index 6798ecf..cced7c9 100644
@@ -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),

drivers/gpu/drm/bridge/adv7511/adv7533.c void adv7533_dsi_power_on(struct adv7511 *adv) · line 128

Quoted 2 lines

drivers/gpu/drm/bridge/adv7511/adv7533.c +2 -0

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

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 <[email protected]>

Cc Archit Taneja <[email protected]>

Cc Laurent Pinchart <[email protected]>

Cc Wolfram Sang <[email protected]>

Cc Srinivas Kandagatla <[email protected]>

Cc "Ville Syrjälä" <[email protected]>

Cc Boris Brezillon <[email protected]>

Cc Andy Green <[email protected]>

Cc Dave Long <[email protected]>

Cc Guodong Xu <[email protected]>

Cc Zhangfei Gao <[email protected]>

Cc Mark Brown <[email protected]>

Cc Lars-Peter Clausen <[email protected]>

Cc Jose Abreu <[email protected]>

Cc [email protected]

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

Hi John,

Thank you for the patches.

Quoted 2 lines

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

Where are the dts changes ?

Quoted 3 lines · Laurent Pinchart

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!

Quoted 4 lines

Quoted 2 lines

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

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

Quoted 3 lines

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 ?

Quoted 11 lines

Quoted 4 lines

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

Quoted 10 lines

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

Quoted 2 lines

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