| Message ID | 1432906281-27698-7-git-send-email-jun.nie@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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? > > 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 > > 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); Where is the implementation for these functions? The fact that they seem to operate on global state indicates that something is wrong wit this API. > +#endif /* __ZX_HDMI_AUDIO_H__ */
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? 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.
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
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 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. > BTW: HDMI driver is far from mature, so you did not see HDMI function > implementation in this patch serial. Sure, no problem.
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.
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.
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>; }; };
On Wed, Jun 03, 2015 at 08:40:31PM +0800, Jun Nie wrote: > 2015-06-03 19:01 GMT+08:00 Mark Brown <broonie@kernel.org>: > > 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? Lars had some comments on the S/PDIF driver. I didn't look at the I2S driver yet, I'll try to do that today.
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.
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 --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);
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