diff mbox

[v3,6/6] ASoC: zx: add zx296702 hdmi codec

Message ID 1432906281-27698-7-git-send-email-jun.nie@linaro.org
State New
Headers show

Commit Message

Jun Nie May 29, 2015, 1:31 p.m. UTC 
Add zx296702 hdmi codec to enable SPDIF and I2S output
via HDMI. The SPDIF/I2S route is exclusive with current
software config and need specify which is valid in
defconfig.

Signed-off-by: Jun Nie <jun.nie@linaro.org>
---
 include/sound/zx_hdmi_audio.h    |   7 +++
 sound/soc/codecs/Kconfig         |   3 +
 sound/soc/codecs/Makefile        |   1 +
 sound/soc/codecs/zx296702_hdmi.c | 121 +++++++++++++++++++++++++++++++++++++++
 4 files changed, 132 insertions(+)
 create mode 100644 include/sound/zx_hdmi_audio.h
 create mode 100644 sound/soc/codecs/zx296702_hdmi.c

Comments

Jun Nie June 1, 2015, 1:42 a.m. UTC | #1 
2015-05-29 23:18 GMT+08:00 Mark Brown <broonie@kernel.org>:
> On Fri, May 29, 2015 at 04:10:54PM +0200, Lars-Peter Clausen wrote:
>> On 05/29/2015 03:31 PM, Jun Nie wrote:
>
>> >Add zx296702 hdmi codec to enable SPDIF and I2S output
>> >via HDMI. The SPDIF/I2S route is exclusive with current
>> >software config and need specify which is valid in
>> >defconfig.
>
>> That's an issue, the hardware configuration should not depend on kernel
>> configuration setting. This makes it impossible to run the same kernel on
>> platforms with conflicting settings. It should be possible to configure this
>> dynamically at boot time based on platform_data/devicetree, etc.
>
>> But is this device real hardware anyway?
It is a ready hardware with HDMI audio configured either from internal
SPDIF or I2S interface. Because DAI and CODEC are totally independent
in ASoC design, so CODEC does not know what DAI interface is active. I
guess I can add a dts property to indicate that from machine dts
config.
>
> See previous discussions - the SoC has an internal HDMI encoder
> connected to a S/PDIF IP which can also be brought out directly as that.
> The solution suggested in the previous discussions was to represent the
> HDMI IP as a CODEC and connect the S/PDIF IP up to it with a machine
> driver, I've not looked at this patch yet though.

So I need implement dai link in machine audio card driver, while not
use simple card and device tree initialization. In this way, I surely
can embed HDMI audio codec in HDMI driver. Is that right?

I had try to initialize audio all from dts, so need a HDMI codec dt
node to connect with DAI. However, I cannot find a way to create an
independent HDMI CODEC dt node because it is brought up from HDMI
driver. If my above understanding is correct, a machine level DAI link
shall resolve this issue with dropping simple card and DAI link in
dts. Thanks for your comments!

BTW: HDMI driver is far from mature, so you did not see HDMI function
implementation in this patch serial.

Jun
Jun Nie June 3, 2015, 1:47 a.m. UTC | #2 
2015-06-03 2:13 GMT+08:00 Mark Brown <broonie@kernel.org>:
> On Mon, Jun 01, 2015 at 09:42:34AM +0800, Jun Nie wrote:
>
>> So I need implement dai link in machine audio card driver, while not
>> use simple card and device tree initialization. In this way, I surely
>> can embed HDMI audio codec in HDMI driver. Is that right?
>
> I'm sorry, I don't entirely follow what you're saying here.
I am guessing you suggest me to implement a ASoC board driver, like
sound/soc/omap/omap3pandora.c. I can link HDMI codec with DAI with
snd_soc_dai_link structure to avoid specify the connection in dts.

>
>> I had try to initialize audio all from dts, so need a HDMI codec dt
>> node to connect with DAI. However, I cannot find a way to create an
>> independent HDMI CODEC dt node because it is brought up from HDMI
>> driver. If my above understanding is correct, a machine level DAI link
>> shall resolve this issue with dropping simple card and DAI link in
>> dts. Thanks for your comments!
>
> If the HDMI encoder is simple I'd expect it to be possible to use it
> with simple-card.  If it isn't then we can look at why.

Most of devices is initialized from devicetree if not all on my board.
So I need fill CODEC/DAI dt node for imple-card in dts. I experience
the difficulty that I cannot create HDMI CODEC dt node. Because HDMI
CODEC shall be part of HDMI driver and be brought up with direct call
to snd_soc_register_codec in HDMI driver probe function per my
understanding. Then I do not have an independent dt node for HDMI
CODEC, thus cannot link the CODEC to DAI with dts information. Do you
see any chance to connect a DAI to an embedded CODEC of HDMI video
device? Thank you!

>
>> BTW: HDMI driver is far from mature, so you did not see HDMI function
>> implementation in this patch serial.
>
> Sure, no problem.
Jun Nie June 3, 2015, 12:40 p.m. UTC | #3 
2015-06-03 19:01 GMT+08:00 Mark Brown <broonie@kernel.org>:
> On Wed, Jun 03, 2015 at 09:47:46AM +0800, Jun Nie wrote:
>> 2015-06-03 2:13 GMT+08:00 Mark Brown <broonie@kernel.org>:
>> > On Mon, Jun 01, 2015 at 09:42:34AM +0800, Jun Nie wrote:
>
>> >> So I need implement dai link in machine audio card driver, while not
>> >> use simple card and device tree initialization. In this way, I surely
>> >> can embed HDMI audio codec in HDMI driver. Is that right?
>
>> > I'm sorry, I don't entirely follow what you're saying here.
>
>> I am guessing you suggest me to implement a ASoC board driver, like
>> sound/soc/omap/omap3pandora.c. I can link HDMI codec with DAI with
>> snd_soc_dai_link structure to avoid specify the connection in dts.
>
> You can do that if you like, but it's also fine to show the link between
> the S/PDIF and HDMI IPs in DT if you like.
>
>> >> I had try to initialize audio all from dts, so need a HDMI codec dt
>> >> node to connect with DAI. However, I cannot find a way to create an
>> >> independent HDMI CODEC dt node because it is brought up from HDMI
>> >> driver. If my above understanding is correct, a machine level DAI link
>> >> shall resolve this issue with dropping simple card and DAI link in
>> >> dts. Thanks for your comments!
>
>> > If the HDMI encoder is simple I'd expect it to be possible to use it
>> > with simple-card.  If it isn't then we can look at why.
>
>> Most of devices is initialized from devicetree if not all on my board.
>> So I need fill CODEC/DAI dt node for imple-card in dts. I experience
>> the difficulty that I cannot create HDMI CODEC dt node. Because HDMI
>> CODEC shall be part of HDMI driver and be brought up with direct call
>> to snd_soc_register_codec in HDMI driver probe function per my
>> understanding. Then I do not have an independent dt node for HDMI
>> CODEC, thus cannot link the CODEC to DAI with dts information. Do you
>> see any chance to connect a DAI to an embedded CODEC of HDMI video
>> device? Thank you!
>
> Are you saying that the HDMI IP doesn't appear in DT at all?  If the
> HDMI IP appears in DT it should be possible to reference it.
Thanks for confirmation. Seems I have much to learn in DT. Will create
a CODEC device in HDMI display driver and feed HDMI device to simple
card as below to have a try. Thanks!
Do you think other patches are OK to merge except the two HDMI patches?

hdmi: hdmi@0x12340000 {
        compatible = "zte,zx296702-hdmi";
        reg = <0x12340000 0x1000>;
     };

sound {
        compatible = "simple-audio-card";
        simple-audio-card,name = "zx296702_snd";
        simple-audio-card,cpu {
                sound-dai = <&spdif0>;
        };

        simple-audio-card,codec {
                sound-dai = <&hdmi>;
        };
};
Jun Nie June 4, 2015, 3:35 a.m. UTC | #4 
2015-06-04 1:58 GMT+08:00 Mark Brown <broonie@kernel.org>:
> On Fri, May 29, 2015 at 09:31:21PM +0800, Jun Nie wrote:
>
>> +int zx_hdmi_audio_cfg(int audio_codec, int audio_way,
>> +                   u32 sample_rate, u32 sample_len);
>> +void zx_hdmi_audio_en(int on);
>
> It's a bit hard to review this properly without knowing what these
> functions do.
>
>> +static int zx_hw_params(struct snd_pcm_substream *substream,
>> +                     struct snd_pcm_hw_params *params,
>> +                     struct snd_soc_dai *dai)
>> +{
>> +#ifdef CONFIG_ZX_HDMI_SND_SPDIF
>> +     zx_hdmi_audio_cfg(1, 0, params_rate(params),
>> +#endif
>> +#ifdef CONFIG_ZX_HDMI_SND_I2S
>> +     zx_hdmi_audio_cfg(1, 1, params_rate(params),
>> +#endif
>> +                       params_physical_width(params));
>> +     return 0;
>> +}
>
> The magic numbers aren't great and it does seem a bit odd to call both
> I2S and S/PDIF versions always - but I guess this will be restructured
> following our discussion in the other thread anyway.

Yes, HDMI audio will be moved into video driver side and I will add a
DT property to indicate active HDMI I2S or SPDIF from machine dts.

Will post patches with taking Lars's comments with dropping HDMI stuff.
diff mbox

Patch

diff --git a/include/sound/zx_hdmi_audio.h b/include/sound/zx_hdmi_audio.h
new file mode 100644
index 0000000..7eb0e81
--- /dev/null
+++ b/include/sound/zx_hdmi_audio.h
@@ -0,0 +1,7 @@ 
+#ifndef __ZX_HDMI_AUDIO_H__
+#define __ZX_HDMI_AUDIO_H__
+
+int zx_hdmi_audio_cfg(int audio_codec, int audio_way,
+		      u32 sample_rate, u32 sample_len);
+void zx_hdmi_audio_en(int on);
+#endif /* __ZX_HDMI_AUDIO_H__ */
diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
index 061c465..1ba34ca 100644
--- a/sound/soc/codecs/Kconfig
+++ b/sound/soc/codecs/Kconfig
@@ -844,6 +844,9 @@  config SND_SOC_WM9712
 config SND_SOC_WM9713
 	tristate
 
+config SND_SOC_ZX_HDMI
+	tristate
+
 # Amp
 config SND_SOC_LM4857
 	tristate
diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile
index abe2d7e..b6c4630 100644
--- a/sound/soc/codecs/Makefile
+++ b/sound/soc/codecs/Makefile
@@ -357,6 +357,7 @@  obj-$(CONFIG_SND_SOC_WM9712)	+= snd-soc-wm9712.o
 obj-$(CONFIG_SND_SOC_WM9713)	+= snd-soc-wm9713.o
 obj-$(CONFIG_SND_SOC_WM_ADSP)	+= snd-soc-wm-adsp.o
 obj-$(CONFIG_SND_SOC_WM_HUBS)	+= snd-soc-wm-hubs.o
+obj-$(CONFIG_SND_SOC_ZX_HDMI)	+= zx296702_hdmi.o
 
 # Amp
 obj-$(CONFIG_SND_SOC_MAX9877)	+= snd-soc-max9877.o
diff --git a/sound/soc/codecs/zx296702_hdmi.c b/sound/soc/codecs/zx296702_hdmi.c
new file mode 100644
index 0000000..b256e54
--- /dev/null
+++ b/sound/soc/codecs/zx296702_hdmi.c
@@ -0,0 +1,121 @@ 
+/*
+ * ALSA SoC codec driver for HDMI audio codecs.
+ * Copyright (C) 2015 Linaro
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ */
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <sound/pcm_params.h>
+#include <sound/soc.h>
+#include <sound/zx_hdmi_audio.h>
+
+#define DRV_NAME "zx-hdmi-codec"
+
+static int zx_hw_params(struct snd_pcm_substream *substream,
+			struct snd_pcm_hw_params *params,
+			struct snd_soc_dai *dai)
+{
+#ifdef CONFIG_ZX_HDMI_SND_SPDIF
+	zx_hdmi_audio_cfg(1, 0, params_rate(params),
+#endif
+#ifdef CONFIG_ZX_HDMI_SND_I2S
+	zx_hdmi_audio_cfg(1, 1, params_rate(params),
+#endif
+			  params_physical_width(params));
+	return 0;
+}
+
+static int zx_trigger(struct snd_pcm_substream *substream, int cmd,
+		      struct snd_soc_dai *dai)
+{
+	switch (cmd) {
+	case SNDRV_PCM_TRIGGER_START:
+	case SNDRV_PCM_TRIGGER_RESUME:
+	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
+#ifdef CONFIG_ZX_HDMI_SND_SPDIF
+		zx_hdmi_audio_en(1);
+#endif
+		break;
+
+	case SNDRV_PCM_TRIGGER_STOP:
+	case SNDRV_PCM_TRIGGER_SUSPEND:
+	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
+#ifdef CONFIG_ZX_HDMI_SND_SPDIF
+		/* Mute HDMI before disabling spdif */
+		zx_hdmi_audio_en(0);
+#endif
+		break;
+
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static const struct snd_soc_dai_ops zx_dai_ops = {
+	.trigger	= zx_trigger,
+	.hw_params	= zx_hw_params,
+};
+
+static struct snd_soc_dai_driver zx_hdmi_codec_dai = {
+	.name = "zx-hdmi-hifi",
+	.playback = {
+		.stream_name = "Playback",
+		.channels_min = 2,
+		.channels_max = 8,
+		.rates = SNDRV_PCM_RATE_32000 |
+			SNDRV_PCM_RATE_44100 | SNDRV_PCM_RATE_48000 |
+			SNDRV_PCM_RATE_88200 | SNDRV_PCM_RATE_96000 |
+			SNDRV_PCM_RATE_176400 | SNDRV_PCM_RATE_192000,
+		.formats = SNDRV_PCM_FMTBIT_S16_LE |
+			SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_S32_LE,
+		.sig_bits = 24,
+	},
+	.ops = &zx_dai_ops,
+};
+
+#ifdef CONFIG_OF
+static const struct of_device_id zx_hdmi_audio_codec_ids[] = {
+	{ .compatible = "zte,hdmi-audio", },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, zx_hdmi_audio_codec_ids);
+#endif
+
+static struct snd_soc_codec_driver zx_hdmi_codec = {
+};
+
+static int zx_hdmi_codec_probe(struct platform_device *pdev)
+{
+	return snd_soc_register_codec(&pdev->dev, &zx_hdmi_codec,
+			&zx_hdmi_codec_dai, 1);
+}
+
+static int zx_hdmi_codec_remove(struct platform_device *pdev)
+{
+	snd_soc_unregister_codec(&pdev->dev);
+	return 0;
+}
+
+static struct platform_driver zx_hdmi_codec_driver = {
+	.driver		= {
+		.name	= DRV_NAME,
+		.of_match_table = of_match_ptr(zx_hdmi_audio_codec_ids),
+	},
+
+	.probe		= zx_hdmi_codec_probe,
+	.remove		= zx_hdmi_codec_remove,
+};
+
+module_platform_driver(zx_hdmi_codec_driver);
+
+MODULE_AUTHOR("Jun Nie <jun.nie@linaro.org>");
+MODULE_DESCRIPTION("ZX296702 ASoC HDMI codec driver");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:" DRV_NAME);