All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH early RFC 0/2] Implement ASoC HDMI codec library
@ 2015-05-13  9:23 Jyri Sarha
  2015-05-13  9:23 ` [PATCH early RFC 1/2] ASoC: hdmi-codec-lib: Add hdmi-codec-lib for external HDMI-encoders Jyri Sarha
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Jyri Sarha @ 2015-05-13  9:23 UTC (permalink / raw)
  To: dri-devel, airlied, linux-omap, alsa-devel
  Cc: peter.ujfalusi, moinejf, broonie, Jyri Sarha, liam.r.girdwood,
	tomi.valkeinen, rmk+kernel

This is on early RFC and should not be merged yet. The idea is just
to share my ideas early on as there has been a lot of development
going on around HDMI audio.

The I2S DAI of the ASoC side patch is usable already, the spdif
support has not been tested and the EDID SADs handling should use
Russel's DRM ELD helper when it is ready.

The tda998x patch is just to demonstrate the usage of the
HDMI-codec-lib. At least the audio related DT-bindings are missing
completely and the configuration is hard coded to work on
Beaglebone-Black.

Jean-Francois, would you consider trying the generic ASoC patch with
your HW, as I can not test the spdif functionality with mine?

The library could also be implemented as a separate platform driver,
but then adding a pointer for private data to struct
snd_soc_dai_driver, snd_soc_codec, or to snd_soc_component would be of
great help.

These patches, my tilcdc refactoring[1], and my latest BCLK fixes for
davinci-mcasp diver [2], can found in a branch that produces a working
HDMI audio on Beaglebone-Black here:

https://github.com/jsarha/linux.git linux-master-bbb-hdmi-20150512

[1] http://lists.freedesktop.org/archives/dri-devel/2015-May/082537.html
[2] http://mailman.alsa-project.org/pipermail/alsa-devel/2015-April/090974.html

Jyri Sarha (2):
  ASoC: hdmi-codec-lib: Add hdmi-codec-lib for external HDMI-encoders
  drm/i2c: tda998x: HACK Implement primitive HDMI audio with ASoC
    hdmi-code-lib

 drivers/gpu/drm/i2c/Kconfig       |   1 +
 drivers/gpu/drm/i2c/tda998x_drv.c | 238 +++++++++++++++++
 include/sound/hdmi-codec-lib.h    | 105 ++++++++
 sound/soc/codecs/Kconfig          |   4 +
 sound/soc/codecs/Makefile         |   2 +
 sound/soc/codecs/hdmi-codec-lib.c | 536 ++++++++++++++++++++++++++++++++++++++
 6 files changed, 886 insertions(+)
 create mode 100644 include/sound/hdmi-codec-lib.h
 create mode 100644 sound/soc/codecs/hdmi-codec-lib.c

-- 
1.9.1

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH early RFC 1/2] ASoC: hdmi-codec-lib: Add hdmi-codec-lib for external HDMI-encoders
  2015-05-13  9:23 [PATCH early RFC 0/2] Implement ASoC HDMI codec library Jyri Sarha
@ 2015-05-13  9:23 ` Jyri Sarha
  2015-05-13  9:23 ` [PATCH early RFC 2/2] drm/i2c: tda998x: HACK Implement primitive HDMI audio with ASoC hdmi-code-lib Jyri Sarha
  2015-05-13 17:42 ` [PATCH early RFC 0/2] Implement ASoC HDMI codec library Jean-Francois Moine
  2 siblings, 0 replies; 5+ messages in thread
From: Jyri Sarha @ 2015-05-13  9:23 UTC (permalink / raw)
  To: dri-devel, airlied, linux-omap, alsa-devel
  Cc: peter.ujfalusi, broonie, Jyri Sarha, liam.r.girdwood,
	tomi.valkeinen, rmk+kernel

The hdmi-codec-lib is a library for registering an ASoC codec under an
external HDMI encoder driver with I2S and/or spdif interface.

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-lib.h    | 105 ++++++++
 sound/soc/codecs/Kconfig          |   4 +
 sound/soc/codecs/Makefile         |   2 +
 sound/soc/codecs/hdmi-codec-lib.c | 536 ++++++++++++++++++++++++++++++++++++++
 4 files changed, 647 insertions(+)
 create mode 100644 include/sound/hdmi-codec-lib.h
 create mode 100644 sound/soc/codecs/hdmi-codec-lib.c

diff --git a/include/sound/hdmi-codec-lib.h b/include/sound/hdmi-codec-lib.h
new file mode 100644
index 0000000..9acf9f7
--- /dev/null
+++ b/include/sound/hdmi-codec-lib.h
@@ -0,0 +1,105 @@
+/*
+ * hdmi-codec-lib.h - HDMI codec library 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_LIB_H__
+#define __HDMI_CODEC_LIB_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 {
+	/* For runtime clock configuration from ASoC machine driver.
+	 * A direct forward from set_sysclk in struct snd_soc_dai_ops.
+	 * Optional */
+	int (*set_clk)(struct device *dev, int clk_id, int freq);
+
+	/* Called when ASoC starts an audio stream setup. The call
+	 * provides an audio abort callback for stoping an ongoing
+	 * stream 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 short audio descriptors from connected HDMI device.
+	 * Optional */
+	int (*get_sads)(struct device *dev, struct cea_sad **sads);
+};
+
+/* HDMI codec initalization data */
+struct hdmi_codec_data {
+	struct device *dev; /* The HDMI encoder registering the codec */
+	const struct hdmi_codec_ops *ops;
+	uint i2s:1;
+	uint spdif:1;
+	int max_i2s_channels;
+};
+
+/* Has to be the first member of the hdmi endcoder's drvdata */
+struct hdmi_codec_drvdata {
+	void *codec_data;
+};
+
+int asoc_hdmi_codec_register(struct hdmi_codec_data *data);
+void asoc_hdmi_codec_unregister(struct device *dev);
+
+#endif /* __HDMI_CODEC_LIB_H__ */
diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
index 061c465..05fabf4 100644
--- a/sound/soc/codecs/Kconfig
+++ b/sound/soc/codecs/Kconfig
@@ -77,6 +77,7 @@ config SND_SOC_ALL_CODECS
 	select SND_SOC_MC13783 if MFD_MC13XXX
 	select SND_SOC_ML26124 if I2C
 	select SND_SOC_HDMI_CODEC
+	select SND_SOC_HDMI_CODEC_LIB
 	select SND_SOC_PCM1681 if I2C
 	select SND_SOC_PCM1792A if SPI_MASTER
 	select SND_SOC_PCM3008
@@ -433,6 +434,9 @@ config SND_SOC_DMIC
 config SND_SOC_HDMI_CODEC
        tristate "HDMI stub CODEC"
 
+config SND_SOC_HDMI_CODEC_LIB
+       tristate "lib for HDMI encoders with i2s or spdif interface"
+
 config SND_SOC_ES8328
 	tristate "Everest Semi ES8328 CODEC"
 
diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile
index abe2d7e..ed1c15d 100644
--- a/sound/soc/codecs/Makefile
+++ b/sound/soc/codecs/Makefile
@@ -70,6 +70,7 @@ snd-soc-max9850-objs := max9850.o
 snd-soc-mc13783-objs := mc13783.o
 snd-soc-ml26124-objs := ml26124.o
 snd-soc-hdmi-codec-objs := hdmi.o
+snd-soc-hdmi-codec-lib-objs := hdmi-codec-lib.o
 snd-soc-pcm1681-objs := pcm1681.o
 snd-soc-pcm1792a-codec-objs := pcm1792a.o
 snd-soc-pcm3008-objs := pcm3008.o
@@ -255,6 +256,7 @@ 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_HDMI_CODEC_LIB)	+= snd-soc-hdmi-codec-lib.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-lib.c b/sound/soc/codecs/hdmi-codec-lib.c
new file mode 100644
index 0000000..5e4e9d8
--- /dev/null
+++ b/sound/soc/codecs/hdmi-codec-lib.c
@@ -0,0 +1,536 @@
+/*
+ * ALSA SoC codec library 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 <sound/core.h>
+#include <sound/pcm.h>
+#include <sound/pcm_params.h>
+#include <sound/soc.h>
+#include <sound/hdmi-codec-lib.h>
+
+struct hdmi_codec_priv {
+	struct hdmi_codec_data 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;
+};
+
+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_I2C = 0,
+	DAI_ID_SPDIF,
+};
+
+static
+struct hdmi_codec_priv *get_priv(struct snd_soc_dai *dai)
+{
+	struct hdmi_codec_drvdata *drvdata =
+		snd_soc_codec_get_drvdata(dai->codec);
+
+	return drvdata->codec_data;
+}
+
+#define CAE_SAD_FORMAT_PCM 1
+#define CAE_SAD_PCM_BYTE2_16BIT (1<<0)
+#define CAE_SAD_PCM_BYTE2_20BIT (1<<1)
+#define CAE_SAD_PCM_BYTE2_24BIT (1<<2)
+
+static int hdmi_codec_sads_constraint(struct snd_pcm_substream *substream,
+				      struct snd_soc_dai *dai,
+				      struct cea_sad *sads, int sad_count)
+{
+	struct snd_pcm_runtime *runtime = substream->runtime;
+	struct hdmi_codec_priv *hcp = get_priv(dai);
+	static const unsigned int hdmi_rates[] = {
+		32000, 44100, 48000, 88200, 96000, 176400, 192000
+	};
+	struct cea_sad *sad = NULL;
+	u_int64_t fmt_mask = 0;
+	int ret, i;
+
+	dev_dbg(hcp->hcd.dev, "%s()\n", __func__);
+
+	/* Just use the first SAD block with PCM support */
+	for (i = 0; i < sad_count; i++) {
+		dev_dbg(hcp->hcd.dev,
+			"%d: format 0x%02x freq 0x%02x byte2 0x%02x\n",
+			i, sads[i].format, sads[i].freq, sads[i].byte2);
+		if (sads[i].format == CAE_SAD_FORMAT_PCM) {
+			sad = &sads[i];
+			break;
+		}
+	}
+
+	if (!sad) {
+		dev_info(hcp->hcd.dev, "%s: No PCM support found\n", __func__);
+		return -EINVAL;
+	}
+
+	hcp->ratec.list = hdmi_rates;
+	hcp->ratec.count = ARRAY_SIZE(hdmi_rates);
+	hcp->ratec.mask = sad->freq;
+	ret = snd_pcm_hw_constraint_list(runtime, 0, SNDRV_PCM_HW_PARAM_RATE,
+					 &hcp->ratec);
+	if (ret)
+		return ret;
+
+	ret = snd_pcm_hw_constraint_minmax(runtime,
+					   SNDRV_PCM_HW_PARAM_CHANNELS,
+					   1, sad->channels + 1);
+	if (ret)
+		return ret;
+
+	/* There is no direct link between I2S format and what is
+	 * being sent to HDMI wire. */
+	if (hcp->daifmt[dai->id].fmt == HDMI_SPDIF) {
+		if (sad->byte2 & CAE_SAD_PCM_BYTE2_16BIT) {
+			fmt_mask |= SNDRV_PCM_FMTBIT_S16_LE;
+			fmt_mask |= SNDRV_PCM_FMTBIT_S16_BE;
+		}
+		if (sad->byte2 & CAE_SAD_PCM_BYTE2_20BIT) {
+			fmt_mask |= SNDRV_PCM_FMTBIT_S20_3LE;
+			fmt_mask |= SNDRV_PCM_FMTBIT_S20_3BE;
+		}
+		if (sad->byte2 & CAE_SAD_PCM_BYTE2_24BIT) {
+			fmt_mask |= SNDRV_PCM_FMTBIT_S24_3LE;
+			fmt_mask |= SNDRV_PCM_FMTBIT_S24_LE;
+			fmt_mask |= SNDRV_PCM_FMTBIT_S24_3BE;
+			fmt_mask |= SNDRV_PCM_FMTBIT_S24_BE;
+		}
+
+		ret = snd_pcm_hw_constraint_mask64(runtime,
+						   SNDRV_PCM_HW_PARAM_FORMAT,
+						   fmt_mask);
+	}
+	return ret;
+}
+
+static void hdmi_codec_abort(struct device *dev)
+{
+	struct hdmi_codec_drvdata *drvdata = dev_get_drvdata(dev);
+	struct hdmi_codec_priv *hcp = drvdata->codec_data;
+
+	dev_dbg(hcp->hcd.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 = get_priv(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 = get_priv(dai);
+	struct cea_sad *sads = NULL;
+	int ret = 0;
+
+	dev_dbg(hcp->hcd.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(hcp->hcd.dev,
+						  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_sads) {
+		ret = hcp->hcd.ops->get_sads(hcp->hcd.dev, &sads);
+		if (ret < 0)
+			return ret;
+
+		ret = hdmi_codec_sads_constraint(substream, dai, sads, ret);
+		kfree(sads);
+		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 = get_priv(dai);
+
+	dev_dbg(hcp->hcd.dev, "%s()\n", __func__);
+
+	mutex_lock(&hcp->current_stream_lock);
+	BUG_ON(hcp->current_stream != substream);
+	hcp->current_stream = NULL;
+	mutex_unlock(&hcp->current_stream_lock);
+
+	hcp->hcd.ops->audio_shutdown(hcp->hcd.dev);
+}
+
+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 = get_priv(dai);
+	struct hdmi_codec_params hp = {
+		.cea = { 0 },
+		.iec = {
+			.status = {
+				IEC958_AES0_CON_NOT_COPYRIGHT,
+				IEC958_AES1_CON_GENERAL,
+				IEC958_AES2_CON_SOURCE_UNSPEC,
+				IEC958_AES3_CON_CLOCK_VARIABLE,
+			},
+			.subcode = { 0 },
+			.pad = 0,
+			.dig_subframe = { 0 },
+		}
+	};
+	int ret;
+
+	dev_dbg(hcp->hcd.dev, "%s()\n", __func__);
+
+	ret = hdmi_codec_new_stream(substream, dai);
+	if (ret)
+		return ret;
+
+	hdmi_audio_infoframe_init(&hp.cea);
+	hp.cea.coding_type = HDMI_AUDIO_CODING_TYPE_PCM;
+	hp.cea.channels = params_channels(params);
+
+	switch (params_width(params)) {
+	case 16:
+		hp.iec.status[4] |= IEC958_AES4_CON_WORDLEN_20_16;
+		hp.cea.sample_size = HDMI_AUDIO_SAMPLE_SIZE_16;
+		break;
+	case 18:
+		hp.iec.status[4] |= IEC958_AES4_CON_WORDLEN_22_18;
+		hp.cea.sample_size = HDMI_AUDIO_SAMPLE_SIZE_20;
+		break;
+	case 20:
+		hp.iec.status[4] |= IEC958_AES4_CON_WORDLEN_24_20;
+		hp.cea.sample_size = HDMI_AUDIO_SAMPLE_SIZE_20;
+		break;
+	case 24:
+	case 32:
+		hp.iec.status[4] |= IEC958_AES4_CON_MAX_WORDLEN_24 |
+			IEC958_AES4_CON_WORDLEN_24_20;
+		hp.cea.sample_size = HDMI_AUDIO_SAMPLE_SIZE_24;
+		break;
+	default:
+		dev_err(dai->dev, "sample width not supported!\n");
+		return -EINVAL;
+	}
+
+	switch (params_rate(params)) {
+	case 32000:
+		hp.iec.status[3] |= IEC958_AES3_CON_FS_32000;
+		hp.cea.sample_frequency = HDMI_AUDIO_SAMPLE_FREQUENCY_32000;
+		break;
+	case 44100:
+		hp.iec.status[3] |= IEC958_AES3_CON_FS_44100;
+		hp.cea.sample_frequency = HDMI_AUDIO_SAMPLE_FREQUENCY_44100;
+		break;
+	case 48000:
+		hp.iec.status[3] |= IEC958_AES3_CON_FS_48000;
+		hp.cea.sample_frequency = HDMI_AUDIO_SAMPLE_FREQUENCY_48000;
+		break;
+	case 88200:
+		hp.iec.status[3] |= IEC958_AES3_CON_FS_88200;
+		hp.cea.sample_frequency = HDMI_AUDIO_SAMPLE_FREQUENCY_88200;
+		break;
+	case 96000:
+		hp.iec.status[3] |= IEC958_AES3_CON_FS_96000;
+		hp.cea.sample_frequency = HDMI_AUDIO_SAMPLE_FREQUENCY_96000;
+		break;
+	case 176400:
+		hp.iec.status[3] |= IEC958_AES3_CON_FS_176400;
+		hp.cea.sample_frequency = HDMI_AUDIO_SAMPLE_FREQUENCY_176400;
+		break;
+	case 192000:
+		hp.iec.status[3] |= IEC958_AES3_CON_FS_192000;
+		hp.cea.sample_frequency = HDMI_AUDIO_SAMPLE_FREQUENCY_192000;
+		break;
+	default:
+		dev_err(dai->dev, "rate not supported!\n");
+		return -EINVAL;
+	}
+	hp.sample_width = params_width(params);
+	hp.sample_rate = params_rate(params);
+	hp.channels = params_channels(params);
+
+	return hcp->hcd.ops->hw_params(hcp->hcd.dev, &hcp->daifmt[dai->id],
+				       &hp);
+}
+
+static int hdmi_codec_set_sysclk(struct snd_soc_dai *dai,
+				 int clk_id, unsigned int freq, int dir)
+{
+	struct hdmi_codec_priv *hcp = get_priv(dai);
+
+	dev_dbg(hcp->hcd.dev, "%s()\n", __func__);
+
+	if (hcp->hcd.ops->set_clk)
+		return hcp->hcd.ops->set_clk(hcp->hcd.dev, clk_id, freq);
+
+	return 0;
+}
+
+static int hdmi_codec_set_fmt(struct snd_soc_dai *dai,
+			      unsigned int fmt)
+{
+	struct hdmi_codec_priv *hcp = get_priv(dai);
+	struct hdmi_codec_daifmt cf = { 0 };
+	int ret = 0;
+
+	dev_dbg(hcp->hcd.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(hcp->hcd.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 = get_priv(dai);
+
+	dev_dbg(hcp->hcd.dev, "%s()\n", __func__);
+
+	if (hcp->hcd.ops->digital_mute)
+		return hcp->hcd.ops->digital_mute(hcp->hcd.dev, 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_sysclk	= hdmi_codec_set_sysclk,
+	.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_I2C,
+	.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),
+};
+
+int asoc_hdmi_codec_register(struct hdmi_codec_data *hcd)
+{
+	struct hdmi_codec_drvdata *drvdata = dev_get_drvdata(hcd->dev);
+	struct hdmi_codec_priv *hcp;
+	int dai_count, i = 0;
+
+	dev_dbg(hcd->dev, "%s()\n", __func__);
+
+	if (!hcd || !hcd->dev || !hcd->ops)
+		return -EINVAL;
+
+	if (!try_module_get(THIS_MODULE))
+		return -ENODEV;
+
+	dai_count = hcd->i2s + hcd->spdif;
+	if (dai_count < 1 || !hcd->ops->hw_params ||
+	    !hcd->ops->audio_shutdown) {
+		dev_err(hcd->dev, "%s: Invalid parameters\n", __func__);
+		module_put(THIS_MODULE);
+		return -EINVAL;
+	}
+
+	hcp = devm_kzalloc(hcd->dev, sizeof(*hcp), GFP_KERNEL);
+	if (!hcp) {
+		module_put(THIS_MODULE);
+		return -ENOMEM;
+	}
+
+	hcp->hcd = *hcd;
+	mutex_init(&hcp->current_stream_lock);
+
+	hcp->daidrv = devm_kzalloc(hcd->dev, dai_count * sizeof(*hcp->daidrv),
+				   GFP_KERNEL);
+	if (!hcp->daidrv) {
+		module_put(THIS_MODULE);
+		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;
+
+	drvdata->codec_data = hcp;
+
+	return snd_soc_register_codec(hcp->hcd.dev, &hdmi_codec, hcp->daidrv,
+				      dai_count);
+}
+EXPORT_SYMBOL_GPL(asoc_hdmi_codec_register);
+
+void asoc_hdmi_codec_unregister(struct device *dev)
+{
+	snd_soc_unregister_codec(dev);
+	module_put(THIS_MODULE);
+}
+EXPORT_SYMBOL_GPL(asoc_hdmi_codec_unregister);
+
+MODULE_AUTHOR("Jyri Sarha <jsarha@ti.com>");
+MODULE_DESCRIPTION("HDMI Codec Library");
+MODULE_LICENSE("GPL");
-- 
1.9.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH early RFC 2/2] drm/i2c: tda998x: HACK Implement primitive HDMI audio with ASoC hdmi-code-lib
  2015-05-13  9:23 [PATCH early RFC 0/2] Implement ASoC HDMI codec library Jyri Sarha
  2015-05-13  9:23 ` [PATCH early RFC 1/2] ASoC: hdmi-codec-lib: Add hdmi-codec-lib for external HDMI-encoders Jyri Sarha
@ 2015-05-13  9:23 ` Jyri Sarha
  2015-05-13 17:42 ` [PATCH early RFC 0/2] Implement ASoC HDMI codec library Jean-Francois Moine
  2 siblings, 0 replies; 5+ messages in thread
From: Jyri Sarha @ 2015-05-13  9:23 UTC (permalink / raw)
  To: dri-devel, airlied, linux-omap, alsa-devel
  Cc: broonie, liam.r.girdwood, peter.ujfalusi, tomi.valkeinen,
	moinejf, rmk+kernel, Jyri Sarha

This patch is to demonstrate how to use the ASoC hdmi-codec-lib to
implement ASoC codec API in tda998x driver.

I do not have proper documentation for tda998x family chips so I lack
the necessary information for making a proper binding for audio part
of the chip. The configuration is hard coded to work on
Beaglebone-Black.

Signed-off-by: Jyri Sarha <jsarha@ti.com>
---
 drivers/gpu/drm/i2c/Kconfig       |   1 +
 drivers/gpu/drm/i2c/tda998x_drv.c | 238 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 239 insertions(+)

diff --git a/drivers/gpu/drm/i2c/Kconfig b/drivers/gpu/drm/i2c/Kconfig
index 22c7ed6..92302e9 100644
--- a/drivers/gpu/drm/i2c/Kconfig
+++ b/drivers/gpu/drm/i2c/Kconfig
@@ -28,6 +28,7 @@ config DRM_I2C_SIL164
 config DRM_I2C_NXP_TDA998X
 	tristate "NXP Semiconductors TDA998X HDMI encoder"
 	default m if DRM_TILCDC
+	select SND_SOC_HDMI_CODEC_LIB if SND_SOC
 	help
 	  Support for NXP Semiconductors TDA998X HDMI encoders.
 
diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
index 5febffd..797b4d5 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -20,6 +20,7 @@
 #include <linux/module.h>
 #include <linux/irq.h>
 #include <sound/asoundef.h>
+#include <sound/hdmi-codec-lib.h>
 
 #include <drm/drmP.h>
 #include <drm/drm_crtc_helper.h>
@@ -31,6 +32,7 @@
 #define DBG(fmt, ...) DRM_DEBUG(fmt"\n", ##__VA_ARGS__)
 
 struct tda998x_priv {
+	struct hdmi_codec_drvdata audio_data;
 	struct i2c_client *cec;
 	struct i2c_client *hdmi;
 	struct mutex mutex;
@@ -44,6 +46,10 @@ struct tda998x_priv {
 	u8 vip_cntrl_2;
 	struct tda998x_encoder_params params;
 
+	struct mutex sads_mutex;
+	struct cea_sad *sads;
+	int sads_count;
+
 	wait_queue_head_t wq_edid;
 	volatile int wq_edid_wait;
 	struct drm_encoder *encoder;
@@ -1115,6 +1121,13 @@ tda998x_encoder_get_modes(struct tda998x_priv *priv,
 	drm_mode_connector_update_edid_property(connector, edid);
 	n = drm_add_edid_modes(connector, edid);
 	priv->is_hdmi_sink = drm_detect_hdmi_monitor(edid);
+
+	mutex_lock(&priv->sads_mutex);
+	kfree(priv->sads);
+	priv->sads = NULL;
+	priv->sads_count = drm_edid_to_sad(edid, &priv->sads);
+	mutex_unlock(&priv->sads_mutex);
+
 	kfree(edid);
 
 	return n;
@@ -1151,6 +1164,14 @@ static void tda998x_destroy(struct tda998x_priv *priv)
 	}
 
 	i2c_unregister_device(priv->cec);
+
+	asoc_hdmi_codec_unregister(&priv->hdmi->dev);
+
+	mutex_lock(&priv->sads_mutex);
+	kfree(priv->sads);
+	priv->sads = NULL;
+	priv->sads_count = -ENODEV;
+	mutex_unlock(&priv->sads_mutex);
 }
 
 /* Slave encoder support */
@@ -1225,6 +1246,217 @@ static struct drm_encoder_slave_funcs tda998x_encoder_slave_funcs = {
 	.set_property = tda998x_encoder_set_property,
 };
 
+static int
+tda998x_configure_audio2(struct tda998x_priv *priv,
+			int mode_clock,
+			int audio_ena,
+			struct hdmi_codec_params *params,
+			struct hdmi_codec_daifmt *daifmt)
+{
+	uint8_t buf[6], clksel_aip, clksel_fs, cts_n, adiv;
+	uint8_t infoframe_buf[HDMI_INFOFRAME_SIZE(AUDIO)];
+	int infoframe_len;
+	uint32_t n;
+
+	infoframe_len = hdmi_audio_infoframe_pack(&params->cea, infoframe_buf,
+						  sizeof(infoframe_buf));
+	if (infoframe_len < 0) {
+		dev_err(&priv->hdmi->dev,
+			"Failed to pack audio infoframe: %d\n",
+			infoframe_len);
+		return infoframe_len;
+	}
+
+	/* Enable audio ports */
+	reg_write(priv, REG_ENA_AP, audio_ena);
+	reg_write(priv, REG_ENA_ACLK, daifmt->fmt == HDMI_SPDIF ? 0 : 1);
+
+	/* Set audio input source */
+	switch (daifmt->fmt) {
+	case HDMI_SPDIF:
+		reg_write(priv, REG_MUX_AP, MUX_AP_SELECT_SPDIF);
+		clksel_aip = AIP_CLKSEL_AIP_SPDIF;
+		clksel_fs = AIP_CLKSEL_FS_FS64SPDIF;
+		cts_n = CTS_N_M(3) | CTS_N_K(3);
+		break;
+
+	case HDMI_I2S:
+		reg_write(priv, REG_MUX_AP, MUX_AP_SELECT_I2S);
+		clksel_aip = AIP_CLKSEL_AIP_I2S;
+		clksel_fs = AIP_CLKSEL_FS_ACLK;
+		switch (params->sample_width) {
+		case 16:
+			cts_n = CTS_N_M(3) | CTS_N_K(1);
+			break;
+		case 18:
+		case 20:
+		case 24:
+			cts_n = CTS_N_M(3) | CTS_N_K(2);
+			break;
+		default:
+		case 32:
+			cts_n = CTS_N_M(3) | CTS_N_K(3);
+			break;
+		}
+		break;
+
+	default:
+		dev_err(&priv->hdmi->dev, "Unsupported I2S format\n");
+		return -EINVAL;
+	}
+
+	reg_write(priv, REG_AIP_CLKSEL, clksel_aip);
+	reg_clear(priv, REG_AIP_CNTRL_0, AIP_CNTRL_0_LAYOUT |
+					AIP_CNTRL_0_ACR_MAN);	/* auto CTS */
+	reg_write(priv, REG_CTS_N, cts_n);
+
+	/*
+	 * Audio input somehow depends on HDMI line rate which is
+	 * related to pixclk. Testing showed that modes with pixclk
+	 * >100MHz need a larger divider while <40MHz need the default.
+	 * There is no detailed info in the datasheet, so we just
+	 * assume 100MHz requires larger divider.
+	 */
+	adiv = AUDIO_DIV_SERCLK_8;
+	if (mode_clock > 100000)
+		adiv++;			/* AUDIO_DIV_SERCLK_16 */
+
+	/* S/PDIF asks for a larger divider */
+	if (daifmt->fmt == HDMI_SPDIF)
+		adiv++;			/* AUDIO_DIV_SERCLK_16 or _32 */
+
+	reg_write(priv, REG_AUDIO_DIV, adiv);
+
+	/*
+	 * This is the approximate value of N, which happens to be
+	 * the recommended values for non-coherent clocks.
+	 */
+	n = 128 * params->sample_rate / 1000;
+
+	/* Write the CTS and N values */
+	buf[0] = 0x44;
+	buf[1] = 0x42;
+	buf[2] = 0x01;
+	buf[3] = n;
+	buf[4] = n >> 8;
+	buf[5] = n >> 16;
+	reg_write_range(priv, REG_ACR_CTS_0, buf, 6);
+
+	/* Set CTS clock reference */
+	reg_write(priv, REG_AIP_CLKSEL, clksel_aip | clksel_fs);
+
+	/* Reset CTS generator */
+	reg_set(priv, REG_AIP_CNTRL_0, AIP_CNTRL_0_RST_CTS);
+	reg_clear(priv, REG_AIP_CNTRL_0, AIP_CNTRL_0_RST_CTS);
+
+	/* Write the channel status */
+	reg_write_range(priv, REG_CH_STAT_B(0), params->iec.status, 4);
+
+	tda998x_audio_mute(priv, true);
+	msleep(20);
+	tda998x_audio_mute(priv, false);
+
+	/* Write the audio information packet */
+	tda998x_write_if(priv, DIP_IF_FLAGS_IF4, REG_IF4_HB0,
+			 infoframe_buf,
+			 infoframe_len);
+	return 0;
+}
+
+static int tda998x_audio_hw_params(struct device *dev,
+				   struct hdmi_codec_daifmt *daifmt,
+				   struct hdmi_codec_params *params)
+{
+	struct tda998x_priv *priv = dev_get_drvdata(dev);
+
+	if (!priv->encoder->crtc)
+		return -ENODEV;
+
+	switch (daifmt->fmt) {
+	case HDMI_I2S:
+		if (daifmt->bit_clk_inv || daifmt->frame_clk_inv ||
+		    daifmt->bit_clk_master || daifmt->frame_clk_master) {
+			dev_err(dev, "%s: Bad flags %d %d %d %d\n", __func__,
+				daifmt->bit_clk_inv, daifmt->frame_clk_inv,
+				daifmt->bit_clk_master,
+				daifmt->frame_clk_master);
+			return -EINVAL;
+		}
+		break;
+	case HDMI_SPDIF:
+		break;
+	default:
+		dev_err(dev, "%s: Invalid format %d\n", __func__, daifmt->fmt);
+		return -EINVAL;
+	}
+
+	return tda998x_configure_audio2(priv,
+					priv->encoder->crtc->hwmode.clock,
+					priv->params.audio_cfg,
+					params,
+					daifmt);
+}
+
+static void tda998x_audio_shutdown(struct device *dev)
+{
+	struct tda998x_priv *priv = dev_get_drvdata(dev);
+
+	reg_write(priv, REG_ENA_AP, 0);
+}
+
+int tda998x_audio_digital_mute(struct device *dev, bool enable)
+{
+	struct tda998x_priv *priv = dev_get_drvdata(dev);
+
+	tda998x_audio_mute(priv, enable);
+
+	return 0;
+}
+
+static int tda998x_audio_get_sads(struct device *dev, struct cea_sad **sads)
+{
+	struct tda998x_priv *priv = dev_get_drvdata(dev);
+	int ret = priv->sads_count;
+
+	mutex_lock(&priv->sads_mutex);
+	if (priv->sads_count > 0) {
+		*sads = kcalloc(priv->sads_count, sizeof(**sads), GFP_KERNEL);
+		if (*sads == NULL) {
+			ret = -ENOMEM;
+			goto out;
+		}
+		memcpy(*sads, priv->sads, priv->sads_count * sizeof(**sads));
+	}
+	mutex_unlock(&priv->sads_mutex);
+
+out:
+	return ret;
+}
+
+static const struct hdmi_codec_ops audio_codec_ops = {
+	.hw_params = tda998x_audio_hw_params,
+	.audio_shutdown = tda998x_audio_shutdown,
+	.digital_mute = tda998x_audio_digital_mute,
+	.get_sads = tda998x_audio_get_sads,
+};
+
+static int tda998x_audio_codec_init(struct tda998x_priv *priv,
+				    struct device *dev)
+{
+	struct hdmi_codec_data codec_data = {
+		.dev = dev,
+		.ops = &audio_codec_ops,
+		.i2s = 1,
+		.spdif = 0,
+		.max_i2s_channels = 2,
+	};
+
+	/* This should be taken from dt or pdata */
+	priv->params.audio_cfg = 0x3;
+
+	return asoc_hdmi_codec_register(&codec_data);
+}
+
 /* I2C driver functions */
 
 static int tda998x_create(struct i2c_client *client, struct tda998x_priv *priv)
@@ -1345,6 +1577,10 @@ static int tda998x_create(struct i2c_client *client, struct tda998x_priv *priv)
 		priv->vip_cntrl_2 = video;
 	}
 
+	mutex_init(&priv->sads_mutex);
+
+	tda998x_audio_codec_init(priv, &client->dev);
+
 	return 0;
 
 fail:
@@ -1375,6 +1611,8 @@ static int tda998x_encoder_init(struct i2c_client *client,
 		return ret;
 	}
 
+	dev_set_drvdata(&client->dev, priv);
+
 	encoder_slave->slave_priv = priv;
 	encoder_slave->slave_funcs = &tda998x_encoder_slave_funcs;
 
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH early RFC 0/2] Implement ASoC HDMI codec library
  2015-05-13  9:23 [PATCH early RFC 0/2] Implement ASoC HDMI codec library Jyri Sarha
  2015-05-13  9:23 ` [PATCH early RFC 1/2] ASoC: hdmi-codec-lib: Add hdmi-codec-lib for external HDMI-encoders Jyri Sarha
  2015-05-13  9:23 ` [PATCH early RFC 2/2] drm/i2c: tda998x: HACK Implement primitive HDMI audio with ASoC hdmi-code-lib Jyri Sarha
@ 2015-05-13 17:42 ` Jean-Francois Moine
  2015-05-15  7:13   ` Jyri Sarha
  2 siblings, 1 reply; 5+ messages in thread
From: Jean-Francois Moine @ 2015-05-13 17:42 UTC (permalink / raw)
  To: Jyri Sarha
  Cc: peter.ujfalusi, alsa-devel, tomi.valkeinen, dri-devel,
	liam.r.girdwood, broonie, rmk+kernel, linux-omap

On Wed, 13 May 2015 12:23:45 +0300
Jyri Sarha <jsarha@ti.com> wrote:

> Jean-Francois, would you consider trying the generic ASoC patch with
> your HW, as I can not test the spdif functionality with mine?

Hi Jyri,

I am not sure to need all the stuff you coded.

My tda998x CODEC is quite empty and it works fine in my system.
If you look at my last patch request ([PATCH v12 6/6] ASoC: tda998x:
add a codec to the HDMI transmitter -
http://mailman.alsa-project.org/pipermail/alsa-devel/2015-May/091758.html),
the code is much smaller than yours and does not ask for a structure
constraint (+/* Has to be the first member of the hdmi endcoder's
drvdata */).

So, I'd rather see a real hdmi codec library, i.e. a set of common
functions as Russell's DRM ELD helper, each specific hdmi codec being
free about the mechanism used for the exchanges with the hdmi
transmitter.

-- 
Ken ar c'hentañ	|	      ** Breizh ha Linux atav! **
Jef		|		http://moinejf.free.fr/
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH early RFC 0/2] Implement ASoC HDMI codec library
  2015-05-13 17:42 ` [PATCH early RFC 0/2] Implement ASoC HDMI codec library Jean-Francois Moine
@ 2015-05-15  7:13   ` Jyri Sarha
  0 siblings, 0 replies; 5+ messages in thread
From: Jyri Sarha @ 2015-05-15  7:13 UTC (permalink / raw)
  To: Jean-Francois Moine
  Cc: dri-devel, airlied, linux-omap, alsa-devel, broonie,
	liam.r.girdwood, peter.ujfalusi, tomi.valkeinen, rmk+kernel

On 05/13/15 20:42, Jean-Francois Moine wrote:
> On Wed, 13 May 2015 12:23:45 +0300
> Jyri Sarha <jsarha@ti.com> wrote:
>
>> Jean-Francois, would you consider trying the generic ASoC patch with
>> your HW, as I can not test the spdif functionality with mine?
>
> Hi Jyri,
>
> I am not sure to need all the stuff you coded.
>

You do not need to use all of the stuff on the ASoC side. At minimum you 
could just implement hw_params() and audio_shutdown() for struct 
hdmi_codec_ops and just use the hw_params() parameters you need. With 
such and implementation the tda998x side would not look too different 
from what you are having now.

> My tda998x CODEC is quite empty and it works fine in my system.
> If you look at my last patch request ([PATCH v12 6/6] ASoC: tda998x:
> add a codec to the HDMI transmitter -

The idea for my lib is to work for all external HDMI encoders with i2s 
and/or spdif interface. And avoid the need to create a new HW specific 
ASoC module and API for each HDMI encoder.

> http://mailman.alsa-project.org/pipermail/alsa-devel/2015-May/091758.html),
> the code is much smaller than yours and does not ask for a structure
> constraint (+/* Has to be the first member of the hdmi endcoder's
> drvdata */).
>

If that is generally considered annoying, I can get around it by adding 
a private_data member to struct snd_soc_dai (Mark, is that Ok?).

With the above change it is simply a matter of choice whether the 
library is actually a platform driver.

> So, I'd rather see a real hdmi codec library, i.e. a set of common
> functions as Russell's DRM ELD helper, each specific hdmi codec being
> free about the mechanism used for the exchanges with the hdmi
> transmitter.
>

Well, this is a real library and these are common functions for all HDMI 
encoder fitting the description. If we want to avoid each encoder from 
adding their own module to ASoC side and implementing their own API, we 
need something similar to what I wrote.

I am happy to do what ever changes are considered necessary, but I think 
I my idea in general is sound.

Of course another option would be just to implement the ASoC component 
within the video side driver and use the ASoC structures directly.

Best regards,
Jyri

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2015-05-15  7:13 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-05-13  9:23 [PATCH early RFC 0/2] Implement ASoC HDMI codec library Jyri Sarha
2015-05-13  9:23 ` [PATCH early RFC 1/2] ASoC: hdmi-codec-lib: Add hdmi-codec-lib for external HDMI-encoders Jyri Sarha
2015-05-13  9:23 ` [PATCH early RFC 2/2] drm/i2c: tda998x: HACK Implement primitive HDMI audio with ASoC hdmi-code-lib Jyri Sarha
2015-05-13 17:42 ` [PATCH early RFC 0/2] Implement ASoC HDMI codec library Jean-Francois Moine
2015-05-15  7:13   ` Jyri Sarha

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.