diff mbox series

[v2,05/12] ASoC: hdmi-codec: Add a prepare hook

Message ID 20210525132354.297468-6-maxime@cerno.tech
State Accepted
Commit 2fef64eec23a0840c97977b16dd8919afaffa876
Headers show
Series drm/vc4: hdmi: Enable Channel Mapping, IEC958, HBR Passthrough using hdmi-codec | expand

Commit Message

Maxime Ripard May 25, 2021, 1:23 p.m. UTC
The IEC958 status bit is usually set by the userspace after hw_params
has been called, so in order to use whatever is set by the userspace, we
need to implement the prepare hook. Let's add it to the hdmi_codec_ops,
and mandate that either prepare or hw_params is implemented.

Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 include/sound/hdmi-codec.h    |  12 +++-
 sound/soc/codecs/hdmi-codec.c | 112 ++++++++++++++++++++++++++--------
 2 files changed, 99 insertions(+), 25 deletions(-)

Comments

Mark Brown May 26, 2021, 10:39 a.m. UTC | #1
On Tue, May 25, 2021 at 03:23:47PM +0200, Maxime Ripard wrote:
> The IEC958 status bit is usually set by the userspace after hw_params

> has been called, so in order to use whatever is set by the userspace, we

> need to implement the prepare hook. Let's add it to the hdmi_codec_ops,

> and mandate that either prepare or hw_params is implemented.


Acked-by: Mark Brown <broonie@kernel.org>
Maxime Ripard May 31, 2021, 9:42 a.m. UTC | #2
Hi Mark, Takashi,

On Wed, May 26, 2021 at 11:39:21AM +0100, Mark Brown wrote:
> On Tue, May 25, 2021 at 03:23:47PM +0200, Maxime Ripard wrote:
> > The IEC958 status bit is usually set by the userspace after hw_params
> > has been called, so in order to use whatever is set by the userspace, we
> > need to implement the prepare hook. Let's add it to the hdmi_codec_ops,
> > and mandate that either prepare or hw_params is implemented.
> 
> Acked-by: Mark Brown <broonie@kernel.org>

It looks like you're both happy with the ALSA/ASoC side, how do you want
to get this merged?

There's a build dependency between the DRM bits and the new hook
introduced in hdmi-codec, would you be ok with merging it through the
drm tree?

Maxime
Takashi Iwai May 31, 2021, 11:12 a.m. UTC | #3
On Mon, 31 May 2021 11:42:13 +0200,
Maxime Ripard wrote:
> 
> Hi Mark, Takashi,
> 
> On Wed, May 26, 2021 at 11:39:21AM +0100, Mark Brown wrote:
> > On Tue, May 25, 2021 at 03:23:47PM +0200, Maxime Ripard wrote:
> > > The IEC958 status bit is usually set by the userspace after hw_params
> > > has been called, so in order to use whatever is set by the userspace, we
> > > need to implement the prepare hook. Let's add it to the hdmi_codec_ops,
> > > and mandate that either prepare or hw_params is implemented.
> > 
> > Acked-by: Mark Brown <broonie@kernel.org>
> 
> It looks like you're both happy with the ALSA/ASoC side, how do you want
> to get this merged?
> 
> There's a build dependency between the DRM bits and the new hook
> introduced in hdmi-codec, would you be ok with merging it through the
> drm tree?

Speaking of ALSA core changes, I'm fine with that.


thanks,

Takashi
Maxime Ripard June 3, 2021, 8:18 a.m. UTC | #4
Hi,

On Tue, Jun 01, 2021 at 01:36:24PM +0100, Mark Brown wrote:
> On Mon, May 31, 2021 at 01:12:17PM +0200, Takashi Iwai wrote:
> > Maxime Ripard wrote:
> 
> > > There's a build dependency between the DRM bits and the new hook
> > > introduced in hdmi-codec, would you be ok with merging it through the
> > > drm tree?
> 
> > Speaking of ALSA core changes, I'm fine with that.
> 
> Yeah, a pull request for the shared bits would be handy in case there's
> some collision with other work.

I guess the easiest then would be for you to merge the patches and send
a PR? Assuming you don't want to pull half of DRM of course :)

Maxime
diff mbox series

Patch

diff --git a/include/sound/hdmi-codec.h b/include/sound/hdmi-codec.h
index 4b3a1d374b90..4fc733c8c570 100644
--- a/include/sound/hdmi-codec.h
+++ b/include/sound/hdmi-codec.h
@@ -65,12 +65,22 @@  struct hdmi_codec_ops {
 
 	/*
 	 * Configures HDMI-encoder for audio stream.
-	 * Mandatory
+	 * Having either prepare or hw_params is mandatory.
 	 */
 	int (*hw_params)(struct device *dev, void *data,
 			 struct hdmi_codec_daifmt *fmt,
 			 struct hdmi_codec_params *hparms);
 
+	/*
+	 * Configures HDMI-encoder for audio stream. Can be called
+	 * multiple times for each setup.
+	 *
+	 * Having either prepare or hw_params is mandatory.
+	 */
+	int (*prepare)(struct device *dev, void *data,
+		       struct hdmi_codec_daifmt *fmt,
+		       struct hdmi_codec_params *hparms);
+
 	/*
 	 * Shuts down the audio stream.
 	 * Mandatory
diff --git a/sound/soc/codecs/hdmi-codec.c b/sound/soc/codecs/hdmi-codec.c
index 5d6324585a31..a67c92032e11 100644
--- a/sound/soc/codecs/hdmi-codec.c
+++ b/sound/soc/codecs/hdmi-codec.c
@@ -481,6 +481,42 @@  static void hdmi_codec_shutdown(struct snd_pcm_substream *substream,
 	mutex_unlock(&hcp->lock);
 }
 
+static int hdmi_codec_fill_codec_params(struct snd_soc_dai *dai,
+					unsigned int sample_width,
+					unsigned int sample_rate,
+					unsigned int channels,
+					struct hdmi_codec_params *hp)
+{
+	struct hdmi_codec_priv *hcp = snd_soc_dai_get_drvdata(dai);
+	int idx;
+
+	/* Select a channel allocation that matches with ELD and pcm channels */
+	idx = hdmi_codec_get_ch_alloc_table_idx(hcp, channels);
+	if (idx < 0) {
+		dev_err(dai->dev, "Not able to map channels to speakers (%d)\n",
+			idx);
+		hcp->chmap_idx = HDMI_CODEC_CHMAP_IDX_UNKNOWN;
+		return idx;
+	}
+
+	memset(hp, 0, sizeof(*hp));
+
+	hdmi_audio_infoframe_init(&hp->cea);
+	hp->cea.channels = channels;
+	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->cea.channel_allocation = hdmi_codec_channel_alloc[idx].ca_id;
+
+	hp->sample_width = sample_width;
+	hp->sample_rate = sample_rate;
+	hp->channels = channels;
+
+	hcp->chmap_idx = hdmi_codec_channel_alloc[idx].ca_id;
+
+	return 0;
+}
+
 static int hdmi_codec_hw_params(struct snd_pcm_substream *substream,
 				struct snd_pcm_hw_params *params,
 				struct snd_soc_dai *dai)
@@ -495,13 +531,24 @@  static int hdmi_codec_hw_params(struct snd_pcm_substream *substream,
 			.dig_subframe = { 0 },
 		}
 	};
-	int ret, idx;
+	int ret;
+
+	if (!hcp->hcd.ops->hw_params)
+		return 0;
 
 	dev_dbg(dai->dev, "%s() width %d rate %d channels %d\n", __func__,
 		params_width(params), params_rate(params),
 		params_channels(params));
 
-	memcpy(hp.iec.status, hcp->iec_status, sizeof(hp->iec_status));
+	ret = hdmi_codec_fill_codec_params(dai,
+					   params_width(params),
+					   params_rate(params),
+					   params_channels(params),
+					   &hp);
+	if (ret < 0)
+		return ret;
+
+	memcpy(hp.iec.status, hcp->iec_status, sizeof(hp.iec.status));
 	ret = snd_pcm_fill_iec958_consumer_hw_params(params, hp.iec.status,
 						     sizeof(hp.iec.status));
 	if (ret < 0) {
@@ -510,32 +557,47 @@  static int hdmi_codec_hw_params(struct snd_pcm_substream *substream,
 		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;
-
-	/* Select a channel allocation that matches with ELD and pcm channels */
-	idx = hdmi_codec_get_ch_alloc_table_idx(hcp, hp.cea.channels);
-	if (idx < 0) {
-		dev_err(dai->dev, "Not able to map channels to speakers (%d)\n",
-			idx);
-		hcp->chmap_idx = HDMI_CODEC_CHMAP_IDX_UNKNOWN;
-		return idx;
-	}
-	hp.cea.channel_allocation = hdmi_codec_channel_alloc[idx].ca_id;
-	hcp->chmap_idx = hdmi_codec_channel_alloc[idx].ca_id;
-
-	hp.sample_width = params_width(params);
-	hp.sample_rate = params_rate(params);
-	hp.channels = params_channels(params);
-
 	cf->bit_fmt = params_format(params);
 	return hcp->hcd.ops->hw_params(dai->dev->parent, hcp->hcd.data,
 				       cf, &hp);
 }
 
+static int hdmi_codec_prepare(struct snd_pcm_substream *substream,
+			      struct snd_soc_dai *dai)
+{
+	struct hdmi_codec_priv *hcp = snd_soc_dai_get_drvdata(dai);
+	struct hdmi_codec_daifmt *cf = dai->playback_dma_data;
+	struct snd_pcm_runtime *runtime = substream->runtime;
+	unsigned int channels = runtime->channels;
+	unsigned int width = snd_pcm_format_width(runtime->format);
+	unsigned int rate = runtime->rate;
+	struct hdmi_codec_params hp;
+	int ret;
+
+	if (!hcp->hcd.ops->prepare)
+		return 0;
+
+	dev_dbg(dai->dev, "%s() width %d rate %d channels %d\n", __func__,
+		width, rate, channels);
+
+	ret = hdmi_codec_fill_codec_params(dai, width, rate, channels, &hp);
+	if (ret < 0)
+		return ret;
+
+	memcpy(hp.iec.status, hcp->iec_status, sizeof(hp.iec.status));
+	ret = snd_pcm_fill_iec958_consumer(runtime, hp.iec.status,
+					   sizeof(hp.iec.status));
+	if (ret < 0) {
+		dev_err(dai->dev, "Creating IEC958 channel status failed %d\n",
+			ret);
+		return ret;
+	}
+
+	cf->bit_fmt = runtime->format;
+	return hcp->hcd.ops->prepare(dai->dev->parent, hcp->hcd.data,
+				     cf, &hp);
+}
+
 static int hdmi_codec_i2s_set_fmt(struct snd_soc_dai *dai,
 				  unsigned int fmt)
 {
@@ -627,6 +689,7 @@  static const struct snd_soc_dai_ops hdmi_codec_i2s_dai_ops = {
 	.startup	= hdmi_codec_startup,
 	.shutdown	= hdmi_codec_shutdown,
 	.hw_params	= hdmi_codec_hw_params,
+	.prepare	= hdmi_codec_prepare,
 	.set_fmt	= hdmi_codec_i2s_set_fmt,
 	.mute_stream	= hdmi_codec_mute,
 };
@@ -917,7 +980,8 @@  static int hdmi_codec_probe(struct platform_device *pdev)
 	}
 
 	dai_count = hcd->i2s + hcd->spdif;
-	if (dai_count < 1 || !hcd->ops || !hcd->ops->hw_params ||
+	if (dai_count < 1 || !hcd->ops ||
+	    (!hcd->ops->hw_params && !hcd->ops->prepare) ||
 	    !hcd->ops->audio_shutdown) {
 		dev_err(dev, "%s: Invalid parameters\n", __func__);
 		return -EINVAL;