* Re: [PATCH RFC v4 3/8] ASoC: hdmi-codec: Add hdmi-codec for external HDMI-encoders [not found] ` <56057A9E.9080602@st.com> @ 2015-09-28 9:01 ` Arnaud Pouliquen 2015-09-28 11:26 ` Russell King - ARM Linux 2015-09-28 12:26 ` Jyri Sarha 0 siblings, 2 replies; 9+ messages in thread From: Arnaud Pouliquen @ 2015-09-28 9:01 UTC (permalink / raw) To: Jyri Sarha, dri-devel, airlied, linux-omap, devicetree, bcousson, alsa-devel, peter.ujfalusi, moinejf, tony, broonie few questions/remarks BR, Arnaud > +static void hdmi_codec_abort(struct device *dev) > +{ > + struct hdmi_codec_priv *hcp = dev_get_drvdata(dev); > + > + dev_dbg(dev, "%s()\n", __func__); > + > + mutex_lock(&hcp->current_stream_lock); > + if (hcp->current_stream && hcp->current_stream->runtime && > + snd_pcm_running(hcp->current_stream)) { > + dev_info(dev, "HDMI audio playback aborted\n"); > + snd_pcm_stream_lock_irq(hcp->current_stream); > + snd_pcm_stop(hcp->current_stream, SNDRV_PCM_STATE_DISCONNECTED); > + snd_pcm_stream_unlock_irq(hcp->current_stream); > + } > + mutex_unlock(&hcp->current_stream_lock); > +} Does driver should stop the stream in case of unplug? This could generate unexpected behavior at application level. Perhaps should be better if this part is managed in DRM driver. if HDMI master, I2S bus should be maintained ON to consume the audio stream until application action. > + > +static int hdmi_codec_hw_params(struct snd_pcm_substream *substream, > + struct snd_pcm_hw_params *params, > + struct snd_soc_dai *dai) > +{ > + struct hdmi_codec_priv *hcp = snd_soc_dai_get_drvdata(dai); > + struct hdmi_codec_params hp = { > + .iec = { > + .status = { 0 }, > + .subcode = { 0 }, > + .pad = 0, > + .dig_subframe = { 0 }, > + } > + }; > + int ret; > + > + dev_dbg(dai->dev, "%s() width %d rate %d channels %d\n", __func__, > + params_width(params), params_rate(params), > + params_channels(params)); > + > + ret = snd_pcm_create_iec958_consumer_hw_params(params, hp.iec.status, > + sizeof(hp.iec.status)); Tell me if i am wrong, but in case of SPDIF format, IEC status is managed by cpu_dai not by the codec. To manage IEC61937 a control should be needed to update IEC status bits... > + if (ret < 0) { > + dev_err(dai->dev, "Creating IEC958 channel status failed %d\n", > + ret); > + return ret; > + } > + > + ret = hdmi_codec_new_stream(substream, dai); > + if (ret) > + return ret; > + > + hdmi_audio_infoframe_init(&hp.cea); > + hp.cea.channels = params_channels(params); > + hp.cea.coding_type = HDMI_AUDIO_CODING_TYPE_STREAM; > + hp.cea.sample_size = HDMI_AUDIO_SAMPLE_SIZE_STREAM; > + hp.cea.sample_frequency = HDMI_AUDIO_SAMPLE_FREQUENCY_STREAM; > + > + hp.sample_width = params_width(params); > + hp.sample_rate = params_rate(params); > + hp.channels = params_channels(params); > + > + return hcp->hcd.ops->hw_params(dai->dev->parent, > &hcp->daifmt[dai->id], > + &hp); > +} > + ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH RFC v4 3/8] ASoC: hdmi-codec: Add hdmi-codec for external HDMI-encoders 2015-09-28 9:01 ` [PATCH RFC v4 3/8] ASoC: hdmi-codec: Add hdmi-codec for external HDMI-encoders Arnaud Pouliquen @ 2015-09-28 11:26 ` Russell King - ARM Linux 2015-09-28 12:21 ` Daniel Vetter 2015-09-28 12:26 ` Jyri Sarha 1 sibling, 1 reply; 9+ messages in thread From: Russell King - ARM Linux @ 2015-09-28 11:26 UTC (permalink / raw) To: Arnaud Pouliquen Cc: devicetree, alsa-devel, liam.r.girdwood, tomi.valkeinen, dri-devel, peter.ujfalusi, tony, broonie, Jyri Sarha, bcousson, linux-omap On Mon, Sep 28, 2015 at 11:01:34AM +0200, Arnaud Pouliquen wrote: > few questions/remarks > BR, > Arnaud > > >+static void hdmi_codec_abort(struct device *dev) > >+{ > >+ struct hdmi_codec_priv *hcp = dev_get_drvdata(dev); > >+ > >+ dev_dbg(dev, "%s()\n", __func__); > >+ > >+ mutex_lock(&hcp->current_stream_lock); > >+ if (hcp->current_stream && hcp->current_stream->runtime && > >+ snd_pcm_running(hcp->current_stream)) { > >+ dev_info(dev, "HDMI audio playback aborted\n"); > >+ snd_pcm_stream_lock_irq(hcp->current_stream); > >+ snd_pcm_stop(hcp->current_stream, SNDRV_PCM_STATE_DISCONNECTED); > >+ snd_pcm_stream_unlock_irq(hcp->current_stream); > >+ } > >+ mutex_unlock(&hcp->current_stream_lock); > >+} > Does driver should stop the stream in case of unplug? > This could generate unexpected behavior at application level. > Perhaps should be better if this part is managed in DRM driver. if HDMI > master, I2S bus should be maintained ON to consume the audio stream until > application action. If it does, that's really horrid. Firstly, do you expect your video playback to stop just because you've unplugged the TV? Maybe you've got a dumb HDMI switch, and you've switched from one input to another to momentarily check something on another input - should the video playback suddenly end up with its audio path being dumped on the floor because of that? Also, as I've said before, there's hardware out there where the SPDIF audio output is routed to more than just the HDMI interface, and the capabilities of HDMI really don't apply in that situation - you may have an AV receiver connected to the SPDIF output which is able to accept much more than the basic audio that most TVs accept. Having stuff restricted to the union of the abilities is far too restrictive. Stopping the audio output because the TV went away in this case is also plain idiotic. SPDIF is something that can be routed to multiple devices simultaneously, and there's no capability or connection reporting involved in it. Imposing the status of one "SPDIF listener" on the entire audio system is unreasonable. > >+ > >+static int hdmi_codec_hw_params(struct snd_pcm_substream *substream, > >+ struct snd_pcm_hw_params *params, > >+ struct snd_soc_dai *dai) > >+{ > >+ struct hdmi_codec_priv *hcp = snd_soc_dai_get_drvdata(dai); > >+ struct hdmi_codec_params hp = { > >+ .iec = { > >+ .status = { 0 }, > >+ .subcode = { 0 }, > >+ .pad = 0, > >+ .dig_subframe = { 0 }, > >+ } > >+ }; > >+ int ret; > >+ > >+ dev_dbg(dai->dev, "%s() width %d rate %d channels %d\n", __func__, > >+ params_width(params), params_rate(params), > >+ params_channels(params)); > >+ > >+ ret = snd_pcm_create_iec958_consumer_hw_params(params, hp.iec.status, > >+ sizeof(hp.iec.status)); > Tell me if i am wrong, but in case of SPDIF format, IEC status is managed by > cpu_dai not by the codec. Correct. I2S needs the IEC958 status programmed into the HDMI interface though, because HDMI is basically SPDIF. -- FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net. _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH RFC v4 3/8] ASoC: hdmi-codec: Add hdmi-codec for external HDMI-encoders 2015-09-28 11:26 ` Russell King - ARM Linux @ 2015-09-28 12:21 ` Daniel Vetter 0 siblings, 0 replies; 9+ messages in thread From: Daniel Vetter @ 2015-09-28 12:21 UTC (permalink / raw) To: Russell King - ARM Linux Cc: devicetree, alsa-devel, peter.ujfalusi, tony, broonie, Arnaud Pouliquen, dri-devel, liam.r.girdwood, tomi.valkeinen, Jyri Sarha, bcousson, linux-omap On Mon, Sep 28, 2015 at 12:26:28PM +0100, Russell King - ARM Linux wrote: > On Mon, Sep 28, 2015 at 11:01:34AM +0200, Arnaud Pouliquen wrote: > > few questions/remarks > > BR, > > Arnaud > > > > >+static void hdmi_codec_abort(struct device *dev) > > >+{ > > >+ struct hdmi_codec_priv *hcp = dev_get_drvdata(dev); > > >+ > > >+ dev_dbg(dev, "%s()\n", __func__); > > >+ > > >+ mutex_lock(&hcp->current_stream_lock); > > >+ if (hcp->current_stream && hcp->current_stream->runtime && > > >+ snd_pcm_running(hcp->current_stream)) { > > >+ dev_info(dev, "HDMI audio playback aborted\n"); > > >+ snd_pcm_stream_lock_irq(hcp->current_stream); > > >+ snd_pcm_stop(hcp->current_stream, SNDRV_PCM_STATE_DISCONNECTED); > > >+ snd_pcm_stream_unlock_irq(hcp->current_stream); > > >+ } > > >+ mutex_unlock(&hcp->current_stream_lock); > > >+} > > Does driver should stop the stream in case of unplug? > > This could generate unexpected behavior at application level. > > Perhaps should be better if this part is managed in DRM driver. if HDMI > > master, I2S bus should be maintained ON to consume the audio stream until > > application action. > > If it does, that's really horrid. Atm the rule for display outputs is that nothing gets yanked until userspace approves, since otherwise compositors get stuck (or fall over with an unexpected -EINVAL from the kernel). The exception is DP MST because the current implementation is a complete hack for DP MST sink lifetimes and that's why we need to synchronously nuke them (which means shutting down everything). I'm surprised not a hole lot more people complain about this ... -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH RFC v4 3/8] ASoC: hdmi-codec: Add hdmi-codec for external HDMI-encoders 2015-09-28 9:01 ` [PATCH RFC v4 3/8] ASoC: hdmi-codec: Add hdmi-codec for external HDMI-encoders Arnaud Pouliquen 2015-09-28 11:26 ` Russell King - ARM Linux @ 2015-09-28 12:26 ` Jyri Sarha 1 sibling, 0 replies; 9+ messages in thread From: Jyri Sarha @ 2015-09-28 12:26 UTC (permalink / raw) To: Arnaud Pouliquen, dri-devel, airlied, linux-omap, devicetree, bcousson, alsa-devel, peter.ujfalusi, moinejf, tony, broonie, liam.r.girdwood, tomi.valkeinen, rmk+kernel On 09/28/15 12:01, Arnaud Pouliquen wrote: > few questions/remarks > BR, > Arnaud > >> +static void hdmi_codec_abort(struct device *dev) >> +{ >> + struct hdmi_codec_priv *hcp = dev_get_drvdata(dev); >> + >> + dev_dbg(dev, "%s()\n", __func__); >> + >> + mutex_lock(&hcp->current_stream_lock); >> + if (hcp->current_stream && hcp->current_stream->runtime && >> + snd_pcm_running(hcp->current_stream)) { >> + dev_info(dev, "HDMI audio playback aborted\n"); >> + snd_pcm_stream_lock_irq(hcp->current_stream); >> + snd_pcm_stop(hcp->current_stream, SNDRV_PCM_STATE_DISCONNECTED); >> + snd_pcm_stream_unlock_irq(hcp->current_stream); >> + } >> + mutex_unlock(&hcp->current_stream_lock); >> +} > Does driver should stop the stream in case of unplug? > This could generate unexpected behavior at application level. > Perhaps should be better if this part is managed in DRM driver. if HDMI > master, I2S bus should be maintained ON to consume the audio stream > until application action. > The whole point of this abort callback is to provide the means for the video side to stop the audio playback in a clean way. The abort callback is given to the video side in startup() callback (ASoC side calls video side). If the video side sees that the playback can not go on it can call the abort callback and the audio playback will abort in standard ALSA way and a properly written applications should not get confused. Nothing is forcing the video side to call the callback ever, if so decided. But if for instance power management stops the i2s clocks when connector is unplugged, then the audio can not go on any more and it should be aborted (actually it would abort in a moment when ALSA realizes that the DMA is not running). >> + >> +static int hdmi_codec_hw_params(struct snd_pcm_substream *substream, >> + struct snd_pcm_hw_params *params, >> + struct snd_soc_dai *dai) >> +{ >> + struct hdmi_codec_priv *hcp = snd_soc_dai_get_drvdata(dai); >> + struct hdmi_codec_params hp = { >> + .iec = { >> + .status = { 0 }, >> + .subcode = { 0 }, >> + .pad = 0, >> + .dig_subframe = { 0 }, >> + } >> + }; >> + int ret; >> + >> + dev_dbg(dai->dev, "%s() width %d rate %d channels %d\n", __func__, >> + params_width(params), params_rate(params), >> + params_channels(params)); >> + >> + ret = snd_pcm_create_iec958_consumer_hw_params(params, >> hp.iec.status, >> + sizeof(hp.iec.status)); > Tell me if i am wrong, but in case of SPDIF format, IEC status is > managed by cpu_dai not by the codec. > To manage IEC61937 a control should be needed to update IEC status bits... > AFAIK yes. However, the video side implementation is free to ignore status bits in the struct hdmi_codec_params and it should normally do so if the format is SPDIF. Of course I could save couple CPU cycles in the codec code if would not even produce the bits when the format is SPDIF. Best regards, Jyri >> + if (ret < 0) { >> + dev_err(dai->dev, "Creating IEC958 channel status failed %d\n", >> + ret); >> + return ret; >> + } >> + >> + ret = hdmi_codec_new_stream(substream, dai); >> + if (ret) >> + return ret; >> + >> + hdmi_audio_infoframe_init(&hp.cea); >> + hp.cea.channels = params_channels(params); >> + hp.cea.coding_type = HDMI_AUDIO_CODING_TYPE_STREAM; >> + hp.cea.sample_size = HDMI_AUDIO_SAMPLE_SIZE_STREAM; >> + hp.cea.sample_frequency = HDMI_AUDIO_SAMPLE_FREQUENCY_STREAM; >> + >> + hp.sample_width = params_width(params); >> + hp.sample_rate = params_rate(params); >> + hp.channels = params_channels(params); >> + >> + return hcp->hcd.ops->hw_params(dai->dev->parent, >> &hcp->daifmt[dai->id], >> + &hp); >> +} >> + _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH RFC v4 0/8] Implement generic ASoC HDMI codec and use it in tda998x @ 2015-09-18 11:06 Jyri Sarha 2015-09-18 11:06 ` [PATCH RFC v4 3/8] ASoC: hdmi-codec: Add hdmi-codec for external HDMI-encoders Jyri Sarha 0 siblings, 1 reply; 9+ messages in thread From: Jyri Sarha @ 2015-09-18 11:06 UTC (permalink / raw) To: dri-devel, airlied, linux-omap, devicetree, bcousson, alsa-devel Cc: peter.ujfalusi, moinejf, tony, broonie, Jyri Sarha, liam.r.girdwood, tomi.valkeinen, rmk+kernel Changes since RFC v3, ASoC side: - Add "ALSA: pcm: add IEC958 channel status helper for hw_params" - Add "tda998x: Improve tda998x_configure_audio() audio related pdata" - use snd_pcm_create_iec958_consumer_hw_params() to construct the stream header - Remove set_clk() callback from hdmi-codec. It is not needed for now. - Refer to stream header in AIF as specified in HDMI standard - Set current_stream to NULL only after video side audio_shutdown() has been called. Avoid potential race if video side attempts to abort audio at the same time. - No need to have video side device pointer in the hdmi codec's pdata as it is found from dev->parent. - Fix hdmi-codec enum: DAI_ID_I2C > DAI_ID_I2S - Improve audio_startup API comment - Make improved checkpatch happy - BUG_ON > WARN_ON - put */ ending the block comment to a separate line DRM side: - Fix tda998x get_eld() locking - Change tda998x audio parameters in pdata to more generic, that can be readily used in tda998x_audio_config() - Rename and restructure audio port related private data members to be more descriptive - Require audio configuration trough ASoC hdmi-codec if HDMI audio is configured trough DT binding. DTS: - Increase McASP fifo usage form 1 to 32 The binding for tda998x is taken from Jean Francois' patch series[1] on the same subject. The implementation of the of-node parsing has some minor changes from my self. Here is what I think at least could or should still be done, but non of that stuff does not sounds critical right now. Missing from tda998x driver side - hdmi_codec_ops audio_startup() implementation for audio abort support - multi channel audio support (I would need specs and preferably some HW to test for this). Missing from ASoC side generic implementation: - channel_allocation handling is completely left for the video side driver, see if ASoC side could help in any way - snd_soc_jack functionality to handle hdmi cable plug/unplug events [1] http://mailman.alsa-project.org/pipermail/alsa-devel/2015-July/095596.html Best regards, Jyri Jean-Francois Moine (1): drm/i2c: tda998x: Add support of a DT graph of ports Jyri Sarha (7): ASoC: hdmi: Remove obsolete dummy HDMI codec ALSA: pcm: add IEC958 channel status helper for hw_params ASoC: hdmi-codec: Add hdmi-codec for external HDMI-encoders drm/i2c: tda998x: Remove include/sound/tda998x.h and fix graph parsing drm/i2c: tda998x: Improve tda998x_configure_audio() audio related pdata drm/i2c: tda998x: Register ASoC HDMI codec for audio functionality ARM: dts: am335x-boneblack: Add HDMI audio support .../devicetree/bindings/drm/i2c/tda998x.txt | 51 +++ arch/arm/boot/dts/am335x-boneblack.dts | 90 ++++- drivers/gpu/drm/armada/armada_drv.c | 25 +- drivers/gpu/drm/i2c/Kconfig | 1 + drivers/gpu/drm/i2c/tda998x_drv.c | 319 +++++++++++++--- include/drm/i2c/tda998x.h | 24 +- include/sound/hdmi-codec.h | 104 ++++++ include/sound/pcm_iec958.h | 2 + sound/core/pcm_iec958.c | 52 ++- sound/soc/codecs/Kconfig | 4 +- sound/soc/codecs/Makefile | 4 +- sound/soc/codecs/hdmi-codec.c | 404 +++++++++++++++++++++ sound/soc/codecs/hdmi.c | 109 ------ 13 files changed, 985 insertions(+), 204 deletions(-) create mode 100644 include/sound/hdmi-codec.h create mode 100644 sound/soc/codecs/hdmi-codec.c delete mode 100644 sound/soc/codecs/hdmi.c -- 1.9.1 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH RFC v4 3/8] ASoC: hdmi-codec: Add hdmi-codec for external HDMI-encoders 2015-09-18 11:06 [PATCH RFC v4 0/8] Implement generic ASoC HDMI codec and use it in tda998x Jyri Sarha @ 2015-09-18 11:06 ` Jyri Sarha [not found] ` <6fe9ab5dff972e6f228259d6818f3f481a11577d.1442572860.git.jsarha-l0cyMroinI0@public.gmane.org> 0 siblings, 1 reply; 9+ messages in thread From: Jyri Sarha @ 2015-09-18 11:06 UTC (permalink / raw) To: dri-devel, airlied, linux-omap, devicetree, bcousson, alsa-devel Cc: peter.ujfalusi, moinejf, tony, broonie, Jyri Sarha, liam.r.girdwood, tomi.valkeinen, rmk+kernel The hdmi-codec is a platform device driver to be registered from drivers of external HDMI encoders with I2S and/or spdif interface. The driver in turn registers an ASoC codec for the HDMI encoder's audio functionality. The structures and definitions in the API header are mostly redundant copies of similar structures in ASoC headers. This is on purpose to avoid direct dependencies to ASoC structures in video side driver. Signed-off-by: Jyri Sarha <jsarha@ti.com> --- include/sound/hdmi-codec.h | 104 +++++++++++ sound/soc/codecs/Kconfig | 6 + sound/soc/codecs/Makefile | 2 + sound/soc/codecs/hdmi-codec.c | 404 ++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 516 insertions(+) create mode 100644 include/sound/hdmi-codec.h create mode 100644 sound/soc/codecs/hdmi-codec.c diff --git a/include/sound/hdmi-codec.h b/include/sound/hdmi-codec.h new file mode 100644 index 0000000..15fe70f --- /dev/null +++ b/include/sound/hdmi-codec.h @@ -0,0 +1,104 @@ +/* + * hdmi-codec.h - HDMI Codec driver API + * + * Copyright (C) 2014 Texas Instruments Incorporated - http://www.ti.com + * + * Author: Jyri Sarha <jsarha@ti.com> + * + * 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. + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + */ + +#ifndef __HDMI_CODEC_H__ +#define __HDMI_CODEC_H__ + +#include <linux/hdmi.h> +#include <drm/drm_edid.h> +#include <sound/asoundef.h> +#include <uapi/sound/asound.h> + +/* + * Protocol between ASoC cpu-dai and HDMI-encoder + */ +struct hdmi_codec_daifmt { + enum { + HDMI_I2S, + HDMI_RIGHT_J, + HDMI_LEFT_J, + HDMI_DSP_A, + HDMI_DSP_B, + HDMI_AC97, + HDMI_SPDIF, + } fmt; + int bit_clk_inv:1; + int frame_clk_inv:1; + int bit_clk_master:1; + int frame_clk_master:1; +}; + +/* + * HDMI audio parameters + */ +struct hdmi_codec_params { + struct hdmi_audio_infoframe cea; + struct snd_aes_iec958 iec; + int sample_rate; + int sample_width; + int channels; +}; + +struct hdmi_codec_ops { + /* + * Called when ASoC starts an audio stream setup. The call + * provides an audio abort callback for stoping an ongoing + * stream from video side driver if the HDMI audio becomes + * unavailable. + * Optional + */ + int (*audio_startup)(struct device *dev, + void (*abort_cb)(struct device *dev)); + + /* + * Configures HDMI-encoder for audio stream. + * Mandatory + */ + int (*hw_params)(struct device *dev, + struct hdmi_codec_daifmt *fmt, + struct hdmi_codec_params *hparms); + + /* + * Shuts down the audio stream. + * Mandatory + */ + void (*audio_shutdown)(struct device *dev); + + /* + * Mute/unmute HDMI audio stream. + * Optional + */ + int (*digital_mute)(struct device *dev, bool enable); + + /* + * Provides EDID-Like-Data from connected HDMI device. + * Optional + */ + int (*get_eld)(struct device *dev, uint8_t *buf, size_t len); +}; + +/* HDMI codec initalization data */ +struct hdmi_codec_pdata { + const struct hdmi_codec_ops *ops; + uint i2s:1; + uint spdif:1; + int max_i2s_channels; +}; + +#define HDMI_CODEC_DRV_NAME "hdmi-audio-codec" + +#endif /* __HDMI_CODEC_H__ */ diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig index 0142396..1e6c7e7 100644 --- a/sound/soc/codecs/Kconfig +++ b/sound/soc/codecs/Kconfig @@ -79,6 +79,7 @@ config SND_SOC_ALL_CODECS select SND_SOC_MAX9877 if I2C select SND_SOC_MC13783 if MFD_MC13XXX select SND_SOC_ML26124 if I2C + select SND_SOC_HDMI_CODEC select SND_SOC_PCM1681 if I2C select SND_SOC_PCM1792A if SPI_MASTER select SND_SOC_PCM3008 @@ -441,6 +442,11 @@ config SND_SOC_BT_SCO config SND_SOC_DMIC tristate +config SND_SOC_HDMI_CODEC + tristate + select SND_PCM_ELD + select SND_PCM_IEC958 + config SND_SOC_ES8328 tristate "Everest Semi ES8328 CODEC" diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile index 7d7cc1b..068d483 100644 --- a/sound/soc/codecs/Makefile +++ b/sound/soc/codecs/Makefile @@ -72,6 +72,7 @@ snd-soc-max98925-objs := max98925.o snd-soc-max9850-objs := max9850.o snd-soc-mc13783-objs := mc13783.o snd-soc-ml26124-objs := ml26124.o +snd-soc-hdmi-codec-objs := hdmi-codec.o snd-soc-pcm1681-objs := pcm1681.o snd-soc-pcm1792a-codec-objs := pcm1792a.o snd-soc-pcm3008-objs := pcm3008.o @@ -263,6 +264,7 @@ obj-$(CONFIG_SND_SOC_MAX98925) += snd-soc-max98925.o obj-$(CONFIG_SND_SOC_MAX9850) += snd-soc-max9850.o obj-$(CONFIG_SND_SOC_MC13783) += snd-soc-mc13783.o obj-$(CONFIG_SND_SOC_ML26124) += snd-soc-ml26124.o +obj-$(CONFIG_SND_SOC_HDMI_CODEC) += snd-soc-hdmi-codec.o obj-$(CONFIG_SND_SOC_PCM1681) += snd-soc-pcm1681.o obj-$(CONFIG_SND_SOC_PCM1792A) += snd-soc-pcm1792a-codec.o obj-$(CONFIG_SND_SOC_PCM3008) += snd-soc-pcm3008.o diff --git a/sound/soc/codecs/hdmi-codec.c b/sound/soc/codecs/hdmi-codec.c new file mode 100644 index 0000000..687332d --- /dev/null +++ b/sound/soc/codecs/hdmi-codec.c @@ -0,0 +1,404 @@ +/* + * ALSA SoC codec for HDMI encoder drivers + * Copyright (C) 2015 Texas Instruments Incorporated - http://www.ti.com/ + * Author: Jyri Sarha <jsarha@ti.com> + * + * 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. + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + */ +#include <linux/module.h> +#include <linux/string.h> +#include <sound/core.h> +#include <sound/pcm.h> +#include <sound/pcm_params.h> +#include <sound/soc.h> +#include <sound/pcm_drm_eld.h> +#include <sound/hdmi-codec.h> +#include <sound/pcm_iec958.h> + +#include <drm/drm_crtc.h> /* This is only to get MAX_ELD_BYTES */ + +struct hdmi_codec_priv { + struct hdmi_codec_pdata hcd; + struct snd_soc_dai_driver *daidrv; + struct hdmi_codec_daifmt daifmt[2]; + struct mutex current_stream_lock; + struct snd_pcm_substream *current_stream; + struct snd_pcm_hw_constraint_list ratec; + uint8_t eld[MAX_ELD_BYTES]; +}; + +static const struct snd_soc_dapm_widget hdmi_widgets[] = { + SND_SOC_DAPM_OUTPUT("TX"), +}; + +static const struct snd_soc_dapm_route hdmi_routes[] = { + { "TX", NULL, "Playback" }, +}; + +enum { + DAI_ID_I2S = 0, + DAI_ID_SPDIF, +}; + +static void hdmi_codec_abort(struct device *dev) +{ + struct hdmi_codec_priv *hcp = dev_get_drvdata(dev); + + dev_dbg(dev, "%s()\n", __func__); + + mutex_lock(&hcp->current_stream_lock); + if (hcp->current_stream && hcp->current_stream->runtime && + snd_pcm_running(hcp->current_stream)) { + dev_info(dev, "HDMI audio playback aborted\n"); + snd_pcm_stream_lock_irq(hcp->current_stream); + snd_pcm_stop(hcp->current_stream, SNDRV_PCM_STATE_DISCONNECTED); + snd_pcm_stream_unlock_irq(hcp->current_stream); + } + mutex_unlock(&hcp->current_stream_lock); +} + +static int hdmi_codec_new_stream(struct snd_pcm_substream *substream, + struct snd_soc_dai *dai) +{ + struct hdmi_codec_priv *hcp = snd_soc_dai_get_drvdata(dai); + int ret = 0; + + mutex_lock(&hcp->current_stream_lock); + if (!hcp->current_stream) { + hcp->current_stream = substream; + } else if (hcp->current_stream != substream) { + dev_err(dai->dev, "Only one simultaneous stream supported!\n"); + ret = -EINVAL; + } + mutex_unlock(&hcp->current_stream_lock); + + return ret; +} + +static int hdmi_codec_startup(struct snd_pcm_substream *substream, + struct snd_soc_dai *dai) +{ + struct hdmi_codec_priv *hcp = snd_soc_dai_get_drvdata(dai); + int ret = 0; + + dev_dbg(dai->dev, "%s()\n", __func__); + + ret = hdmi_codec_new_stream(substream, dai); + if (ret) + return ret; + + if (hcp->hcd.ops->audio_startup) { + ret = hcp->hcd.ops->audio_startup(dai->dev->parent, + hdmi_codec_abort); + if (ret) { + mutex_lock(&hcp->current_stream_lock); + hcp->current_stream = NULL; + mutex_unlock(&hcp->current_stream_lock); + return ret; + } + } + + if (hcp->hcd.ops->get_eld) { + ret = hcp->hcd.ops->get_eld(dai->dev->parent, hcp->eld, + sizeof(hcp->eld)); + + if (!ret) { + ret = snd_pcm_hw_constraint_eld(substream->runtime, + hcp->eld); + if (ret) + return ret; + } + } + return 0; +} + +static void hdmi_codec_shutdown(struct snd_pcm_substream *substream, + struct snd_soc_dai *dai) +{ + struct hdmi_codec_priv *hcp = snd_soc_dai_get_drvdata(dai); + + dev_dbg(dai->dev, "%s()\n", __func__); + + WARN_ON(hcp->current_stream != substream); + + hcp->hcd.ops->audio_shutdown(dai->dev->parent); + + mutex_lock(&hcp->current_stream_lock); + hcp->current_stream = NULL; + mutex_unlock(&hcp->current_stream_lock); +} + +static int hdmi_codec_hw_params(struct snd_pcm_substream *substream, + struct snd_pcm_hw_params *params, + struct snd_soc_dai *dai) +{ + struct hdmi_codec_priv *hcp = snd_soc_dai_get_drvdata(dai); + struct hdmi_codec_params hp = { + .iec = { + .status = { 0 }, + .subcode = { 0 }, + .pad = 0, + .dig_subframe = { 0 }, + } + }; + int ret; + + dev_dbg(dai->dev, "%s() width %d rate %d channels %d\n", __func__, + params_width(params), params_rate(params), + params_channels(params)); + + ret = snd_pcm_create_iec958_consumer_hw_params(params, hp.iec.status, + sizeof(hp.iec.status)); + if (ret < 0) { + dev_err(dai->dev, "Creating IEC958 channel status failed %d\n", + ret); + return ret; + } + + ret = hdmi_codec_new_stream(substream, dai); + if (ret) + return ret; + + hdmi_audio_infoframe_init(&hp.cea); + hp.cea.channels = params_channels(params); + hp.cea.coding_type = HDMI_AUDIO_CODING_TYPE_STREAM; + hp.cea.sample_size = HDMI_AUDIO_SAMPLE_SIZE_STREAM; + hp.cea.sample_frequency = HDMI_AUDIO_SAMPLE_FREQUENCY_STREAM; + + hp.sample_width = params_width(params); + hp.sample_rate = params_rate(params); + hp.channels = params_channels(params); + + return hcp->hcd.ops->hw_params(dai->dev->parent, &hcp->daifmt[dai->id], + &hp); +} + +static int hdmi_codec_set_fmt(struct snd_soc_dai *dai, + unsigned int fmt) +{ + struct hdmi_codec_priv *hcp = snd_soc_dai_get_drvdata(dai); + struct hdmi_codec_daifmt cf = { 0 }; + int ret = 0; + + dev_dbg(dai->dev, "%s()\n", __func__); + + if (dai->id == DAI_ID_SPDIF) { + cf.fmt = HDMI_SPDIF; + } else { + switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) { + case SND_SOC_DAIFMT_CBM_CFM: + cf.bit_clk_master = 1; + cf.frame_clk_master = 1; + break; + case SND_SOC_DAIFMT_CBS_CFM: + cf.frame_clk_master = 1; + break; + case SND_SOC_DAIFMT_CBM_CFS: + cf.bit_clk_master = 1; + break; + case SND_SOC_DAIFMT_CBS_CFS: + break; + default: + return -EINVAL; + } + + switch (fmt & SND_SOC_DAIFMT_INV_MASK) { + case SND_SOC_DAIFMT_NB_NF: + break; + case SND_SOC_DAIFMT_NB_IF: + cf.frame_clk_inv = 1; + break; + case SND_SOC_DAIFMT_IB_NF: + cf.bit_clk_inv = 1; + break; + case SND_SOC_DAIFMT_IB_IF: + cf.frame_clk_inv = 1; + cf.bit_clk_inv = 1; + break; + } + + switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) { + case SND_SOC_DAIFMT_I2S: + cf.fmt = HDMI_I2S; + break; + case SND_SOC_DAIFMT_DSP_A: + cf.fmt = HDMI_DSP_A; + break; + case SND_SOC_DAIFMT_DSP_B: + cf.fmt = HDMI_DSP_B; + break; + case SND_SOC_DAIFMT_RIGHT_J: + cf.fmt = HDMI_RIGHT_J; + break; + case SND_SOC_DAIFMT_LEFT_J: + cf.fmt = HDMI_LEFT_J; + break; + case SND_SOC_DAIFMT_AC97: + cf.fmt = HDMI_AC97; + break; + default: + dev_err(dai->dev, "Invalid DAI interface format\n"); + return -EINVAL; + } + } + + hcp->daifmt[dai->id] = cf; + + return ret; +} + +static int hdmi_codec_digital_mute(struct snd_soc_dai *dai, int mute) +{ + struct hdmi_codec_priv *hcp = snd_soc_dai_get_drvdata(dai); + + dev_dbg(dai->dev, "%s()\n", __func__); + + if (hcp->hcd.ops->digital_mute) + return hcp->hcd.ops->digital_mute(dai->dev->parent, mute); + + return 0; +} + +static const struct snd_soc_dai_ops hdmi_dai_ops = { + .startup = hdmi_codec_startup, + .shutdown = hdmi_codec_shutdown, + .hw_params = hdmi_codec_hw_params, + .set_fmt = hdmi_codec_set_fmt, + .digital_mute = hdmi_codec_digital_mute, +}; + + +#define HDMI_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) + +#define SPDIF_FORMATS (SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S16_BE |\ + SNDRV_PCM_FMTBIT_S20_3LE | SNDRV_PCM_FMTBIT_S20_3BE |\ + SNDRV_PCM_FMTBIT_S24_3LE | SNDRV_PCM_FMTBIT_S24_3BE |\ + SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_S24_BE) + +#define I2S_FORMATS (SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S16_BE |\ + SNDRV_PCM_FMTBIT_S18_3LE | SNDRV_PCM_FMTBIT_S18_3BE |\ + SNDRV_PCM_FMTBIT_S20_3LE | SNDRV_PCM_FMTBIT_S20_3BE |\ + SNDRV_PCM_FMTBIT_S24_3LE | SNDRV_PCM_FMTBIT_S24_3BE |\ + SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_S24_BE |\ + SNDRV_PCM_FMTBIT_S32_LE | SNDRV_PCM_FMTBIT_S32_BE) + +static struct snd_soc_dai_driver hdmi_i2s_dai = { + .name = "i2s-hifi", + .id = DAI_ID_I2S, + .playback = { + .stream_name = "Playback", + .channels_min = 2, + .channels_max = 8, + .rates = HDMI_RATES, + .formats = I2S_FORMATS, + .sig_bits = 24, + }, + .ops = &hdmi_dai_ops, +}; + +static const struct snd_soc_dai_driver hdmi_spdif_dai = { + .name = "spdif-hifi", + .id = DAI_ID_SPDIF, + .playback = { + .stream_name = "Playback", + .channels_min = 2, + .channels_max = 2, + .rates = HDMI_RATES, + .formats = SPDIF_FORMATS, + }, + .ops = &hdmi_dai_ops, +}; + +static struct snd_soc_codec_driver hdmi_codec = { + .dapm_widgets = hdmi_widgets, + .num_dapm_widgets = ARRAY_SIZE(hdmi_widgets), + .dapm_routes = hdmi_routes, + .num_dapm_routes = ARRAY_SIZE(hdmi_routes), +}; + +static int hdmi_codec_probe(struct platform_device *pdev) +{ + struct hdmi_codec_pdata *hcd = pdev->dev.platform_data; + struct device *dev = &pdev->dev; + struct hdmi_codec_priv *hcp; + int dai_count, i = 0; + int ret; + + dev_dbg(dev, "%s()\n", __func__); + + if (!hcd) { + dev_err(dev, "%s: No plalform data\n", __func__); + return -EINVAL; + } + + dai_count = hcd->i2s + hcd->spdif; + if (dai_count < 1 || !hcd->ops || !hcd->ops->hw_params || + !hcd->ops->audio_shutdown) { + dev_err(dev, "%s: Invalid parameters\n", __func__); + return -EINVAL; + } + + hcp = devm_kzalloc(dev, sizeof(*hcp), GFP_KERNEL); + if (!hcp) + return -ENOMEM; + + hcp->hcd = *hcd; + mutex_init(&hcp->current_stream_lock); + + hcp->daidrv = devm_kzalloc(dev, dai_count * sizeof(*hcp->daidrv), + GFP_KERNEL); + if (!hcp->daidrv) + return -ENOMEM; + + if (hcd->i2s) { + hcp->daidrv[i] = hdmi_i2s_dai; + hcp->daidrv[i].playback.channels_max = + hcd->max_i2s_channels; + i++; + } + + if (hcd->spdif) + hcp->daidrv[i] = hdmi_spdif_dai; + + ret = snd_soc_register_codec(dev, &hdmi_codec, hcp->daidrv, + dai_count); + if (ret) { + dev_err(dev, "%s: snd_soc_register_codec() failed (%d)\n", + __func__, ret); + return ret; + } + + dev_set_drvdata(dev, hcp); + return 0; +} + +static int hdmi_codec_remove(struct platform_device *pdev) +{ + snd_soc_unregister_codec(&pdev->dev); + return 0; +} + +static struct platform_driver hdmi_codec_driver = { + .driver = { + .name = HDMI_CODEC_DRV_NAME, + }, + .probe = hdmi_codec_probe, + .remove = hdmi_codec_remove, +}; + +module_platform_driver(hdmi_codec_driver); + +MODULE_AUTHOR("Jyri Sarha <jsarha@ti.com>"); +MODULE_DESCRIPTION("HDMI Audio Codec Driver"); +MODULE_LICENSE("GPL"); +MODULE_ALIAS("platform:" HDMI_CODEC_DRV_NAME); -- 1.9.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
[parent not found: <6fe9ab5dff972e6f228259d6818f3f481a11577d.1442572860.git.jsarha-l0cyMroinI0@public.gmane.org>]
* Re: [PATCH RFC v4 3/8] ASoC: hdmi-codec: Add hdmi-codec for external HDMI-encoders [not found] ` <6fe9ab5dff972e6f228259d6818f3f481a11577d.1442572860.git.jsarha-l0cyMroinI0@public.gmane.org> @ 2015-09-19 17:54 ` Mark Brown 2015-09-21 9:31 ` Russell King - ARM Linux 0 siblings, 1 reply; 9+ messages in thread From: Mark Brown @ 2015-09-19 17:54 UTC (permalink / raw) To: Jyri Sarha Cc: dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, airlied-cv59FeDIM0c, linux-omap-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA, bcousson-rdvid1DuHRBWk0Htik3J/w, alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw, tony-4v6yS6AI5VpBDgjK7y7TUQ, liam.r.girdwood-VuQAYsv1563Yd54FQh9/CA, peter.ujfalusi-l0cyMroinI0, tomi.valkeinen-l0cyMroinI0, moinejf-GANU6spQydw, rmk+kernel-lFZ/pmaqli7XmaaqVzeoHQ [-- Attachment #1: Type: text/plain, Size: 1331 bytes --] On Fri, Sep 18, 2015 at 02:06:40PM +0300, Jyri Sarha wrote: > +#define SPDIF_FORMATS (SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S16_BE |\ > + SNDRV_PCM_FMTBIT_S20_3LE | SNDRV_PCM_FMTBIT_S20_3BE |\ > + SNDRV_PCM_FMTBIT_S24_3LE | SNDRV_PCM_FMTBIT_S24_3BE |\ > + SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_S24_BE) > + > +#define I2S_FORMATS (SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S16_BE |\ > + SNDRV_PCM_FMTBIT_S18_3LE | SNDRV_PCM_FMTBIT_S18_3BE |\ > + SNDRV_PCM_FMTBIT_S20_3LE | SNDRV_PCM_FMTBIT_S20_3BE |\ > + SNDRV_PCM_FMTBIT_S24_3LE | SNDRV_PCM_FMTBIT_S24_3BE |\ > + SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_S24_BE |\ > + SNDRV_PCM_FMTBIT_S32_LE | SNDRV_PCM_FMTBIT_S32_BE) I'm not sure how abstracted this I2S and S/PDIF DAI business is - it doesn't feel like something that's going to be a property of all HDMI devices, and the specific sets of formats above cause me to raise a bit of an eyebrow. Should this be an interface where the HDMI chip registers multiple interfaces if it has them instead of automatically getting this split as is? We can probably skip more specific constraint stuff I guess on the basis that things tend to support the full range of HDMI functionality, or at least that can be added later? Otherwise this looks mostly OK on a first pass, thanks for working on it. [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 473 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH RFC v4 3/8] ASoC: hdmi-codec: Add hdmi-codec for external HDMI-encoders 2015-09-19 17:54 ` Mark Brown @ 2015-09-21 9:31 ` Russell King - ARM Linux 2015-09-21 13:41 ` Jyri Sarha 0 siblings, 1 reply; 9+ messages in thread From: Russell King - ARM Linux @ 2015-09-21 9:31 UTC (permalink / raw) To: Mark Brown Cc: devicetree, alsa-devel, peter.ujfalusi, dri-devel, liam.r.girdwood, tony, tomi.valkeinen, Jyri Sarha, bcousson, linux-omap On Sat, Sep 19, 2015 at 10:54:51AM -0700, Mark Brown wrote: > On Fri, Sep 18, 2015 at 02:06:40PM +0300, Jyri Sarha wrote: > > +#define SPDIF_FORMATS (SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S16_BE |\ > > + SNDRV_PCM_FMTBIT_S20_3LE | SNDRV_PCM_FMTBIT_S20_3BE |\ > > + SNDRV_PCM_FMTBIT_S24_3LE | SNDRV_PCM_FMTBIT_S24_3BE |\ > > + SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_S24_BE) > > + > > +#define I2S_FORMATS (SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S16_BE |\ > > + SNDRV_PCM_FMTBIT_S18_3LE | SNDRV_PCM_FMTBIT_S18_3BE |\ > > + SNDRV_PCM_FMTBIT_S20_3LE | SNDRV_PCM_FMTBIT_S20_3BE |\ > > + SNDRV_PCM_FMTBIT_S24_3LE | SNDRV_PCM_FMTBIT_S24_3BE |\ > > + SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_S24_BE |\ > > + SNDRV_PCM_FMTBIT_S32_LE | SNDRV_PCM_FMTBIT_S32_BE) > > I'm not sure how abstracted this I2S and S/PDIF DAI business is - it > doesn't feel like something that's going to be a property of all HDMI > devices, and the specific sets of formats above cause me to raise a bit > of an eyebrow. Should this be an interface where the HDMI chip > registers multiple interfaces if it has them instead of automatically > getting this split as is? The inclusion of the 32-bit formats does raise an eyebrow here too. Audio transmission across the HDMI link is S/PDIF, supporting up to 24-bit uncompressed audio (aka L-PCM). The device may accept 32 bit I2S, but it would have to be truncated to 24 bit before transmitting it to the sink. This should be mentioned somewhere. -- FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net. _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH RFC v4 3/8] ASoC: hdmi-codec: Add hdmi-codec for external HDMI-encoders 2015-09-21 9:31 ` Russell King - ARM Linux @ 2015-09-21 13:41 ` Jyri Sarha 2015-09-21 17:18 ` Mark Brown 0 siblings, 1 reply; 9+ messages in thread From: Jyri Sarha @ 2015-09-21 13:41 UTC (permalink / raw) To: Russell King - ARM Linux, Mark Brown Cc: devicetree, alsa-devel, peter.ujfalusi, dri-devel, liam.r.girdwood, tony, tomi.valkeinen, bcousson, linux-omap On 09/21/15 12:31, Russell King - ARM Linux wrote: > On Sat, Sep 19, 2015 at 10:54:51AM -0700, Mark Brown wrote: >> On Fri, Sep 18, 2015 at 02:06:40PM +0300, Jyri Sarha wrote: >>> +#define SPDIF_FORMATS (SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S16_BE |\ >>> + SNDRV_PCM_FMTBIT_S20_3LE | SNDRV_PCM_FMTBIT_S20_3BE |\ >>> + SNDRV_PCM_FMTBIT_S24_3LE | SNDRV_PCM_FMTBIT_S24_3BE |\ >>> + SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_S24_BE) >>> + >>> +#define I2S_FORMATS (SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S16_BE |\ >>> + SNDRV_PCM_FMTBIT_S18_3LE | SNDRV_PCM_FMTBIT_S18_3BE |\ >>> + SNDRV_PCM_FMTBIT_S20_3LE | SNDRV_PCM_FMTBIT_S20_3BE |\ >>> + SNDRV_PCM_FMTBIT_S24_3LE | SNDRV_PCM_FMTBIT_S24_3BE |\ >>> + SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_S24_BE |\ >>> + SNDRV_PCM_FMTBIT_S32_LE | SNDRV_PCM_FMTBIT_S32_BE) >> >> I'm not sure how abstracted this I2S and S/PDIF DAI business is - it >> doesn't feel like something that's going to be a property of all HDMI >> devices, and the specific sets of formats above cause me to raise a bit >> of an eyebrow. Should this be an interface where the HDMI chip >> registers multiple interfaces if it has them instead of automatically >> getting this split as is? > > The inclusion of the 32-bit formats does raise an eyebrow here too. > Audio transmission across the HDMI link is S/PDIF, supporting up to > 24-bit uncompressed audio (aka L-PCM). > > The device may accept 32 bit I2S, but it would have to be truncated to > 24 bit before transmitting it to the sink. This should be mentioned > somewhere. > There is ".sig_bits = 24" in hdmi_i2s_dai, but I can add an explicit comment about it. We needed 32bit format in practice until Peter got 24_LE properly working with McASP. I just thought the may still be some platforms out there that can not produce 24bit i2s samples for some reason, but work just fine with 32bit samples. Those platforms would be limited to less than 24bit precision without 32bit formats. But as said, it is not critical to us any more and I can drop the 32bit formats as well. BR, Jyri _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH RFC v4 3/8] ASoC: hdmi-codec: Add hdmi-codec for external HDMI-encoders 2015-09-21 13:41 ` Jyri Sarha @ 2015-09-21 17:18 ` Mark Brown 0 siblings, 0 replies; 9+ messages in thread From: Mark Brown @ 2015-09-21 17:18 UTC (permalink / raw) To: Jyri Sarha Cc: devicetree, alsa-devel, Russell King - ARM Linux, peter.ujfalusi, moinejf, airlied, dri-devel, liam.r.girdwood, tony, tomi.valkeinen, bcousson, linux-omap [-- Attachment #1.1: Type: text/plain, Size: 1098 bytes --] On Mon, Sep 21, 2015 at 04:41:20PM +0300, Jyri Sarha wrote: > On 09/21/15 12:31, Russell King - ARM Linux wrote: > >The device may accept 32 bit I2S, but it would have to be truncated to > >24 bit before transmitting it to the sink. This should be mentioned > >somewhere. > There is ".sig_bits = 24" in hdmi_i2s_dai, but I can add an explicit comment > about it. > We needed 32bit format in practice until Peter got 24_LE properly working > with McASP. I just thought the may still be some platforms out there that > can not produce 24bit i2s samples for some reason, but work just fine with > 32bit samples. Those platforms would be limited to less than 24bit precision > without 32bit formats. But as said, it is not critical to us any more and I > can drop the 32bit formats as well. Yes, this is currently a bit messy - we should really be able to allow S24_LE and S32 to interoperate better without drivers having to know about it since physically they are essentially compatible in memory. I'm pretty much OK with it for now since even with that framework support it should be harmless. [-- Attachment #1.2: Digital signature --] [-- Type: application/pgp-signature, Size: 473 bytes --] [-- Attachment #2: Type: text/plain, Size: 0 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2015-09-28 12:26 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- [not found] <mailman.310.1442574427.844.alsa-devel@alsa-project.org> [not found] ` <56057A9E.9080602@st.com> 2015-09-28 9:01 ` [PATCH RFC v4 3/8] ASoC: hdmi-codec: Add hdmi-codec for external HDMI-encoders Arnaud Pouliquen 2015-09-28 11:26 ` Russell King - ARM Linux 2015-09-28 12:21 ` Daniel Vetter 2015-09-28 12:26 ` Jyri Sarha 2015-09-18 11:06 [PATCH RFC v4 0/8] Implement generic ASoC HDMI codec and use it in tda998x Jyri Sarha 2015-09-18 11:06 ` [PATCH RFC v4 3/8] ASoC: hdmi-codec: Add hdmi-codec for external HDMI-encoders Jyri Sarha [not found] ` <6fe9ab5dff972e6f228259d6818f3f481a11577d.1442572860.git.jsarha-l0cyMroinI0@public.gmane.org> 2015-09-19 17:54 ` Mark Brown 2015-09-21 9:31 ` Russell King - ARM Linux 2015-09-21 13:41 ` Jyri Sarha 2015-09-21 17:18 ` Mark Brown
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).