diff mbox

[1/2,v3] drm/bridge: adv7511: Add Audio support.

Message ID 1473204158-25145-2-git-send-email-john.stultz@linaro.org (mailing list archive)
State New, archived
Headers show

Commit Message

John Stultz Sept. 6, 2016, 11:22 p.m. UTC
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

Comments

Lars-Peter Clausen Sept. 16, 2016, 10:51 a.m. UTC | #1
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,
> +};
John Stultz Sept. 16, 2016, 11:08 p.m. UTC | #2
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
diff mbox

Patch

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