diff mbox

[v4,05/14] ASoC: hdac_hdmi: Apply constraints based on ELD

Message ID 1449677781-7997-5-git-send-email-subhransu.s.prusty@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Subhransu S. Prusty Dec. 9, 2015, 4:16 p.m. UTC 
Uses the drm eld core framework to apply rate and channel

Also compute the format to be set based on ELD.

Even though the channel constraint is based on ELD, infoframe
is set with stereo only. Multichannel support will be added
later.

Signed-off-by: Subhransu S. Prusty <subhransu.s.prusty@intel.com>
Signed-off-by: Vinod Koul <vinod.koul@intel.com>
---
 sound/soc/codecs/Kconfig     |  1 +
 sound/soc/codecs/hdac_hdmi.c | 48 +++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 46 insertions(+), 3 deletions(-)

Comments

Mark Brown Jan. 8, 2016, 2:09 p.m. UTC | #1 
On Wed, Dec 09, 2015 at 09:46:12PM +0530, Subhransu S. Prusty wrote:

> +static int hdac_hdmi_eld_limit_formats(struct snd_pcm_runtime *runtime,
> +						void *eld)
> +{
> +	u64 formats = SNDRV_PCM_FMTBIT_S16;
> +	int i;
> +	const u8 *sad, *eld_buf = eld;
> +
> +	sad = drm_eld_sad(eld_buf);
> +	if (!sad)
> +		goto format_constraint;
> +
> +	for (i = drm_eld_sad_count(eld_buf); i > 0; i--, sad += 3) {
> +		if (sad_format(sad) == 1) { /* AUDIO_CODING_TYPE_LPCM */

	switch (sad_format(sad))  {
	case AUDIO_CODING_TYPE_LPCM:

> +
> +			/* 20 bit and 24 bit */
> +			if (sad_sample_bits_lpcm(sad) & 0x6)
> +				formats |= SNDRV_PCM_FMTBIT_S32;
> +		}
> +	}

I really don't have a clear idea what the above is supposed to do or why
this is specific to HDAC and not handled...

> -	return 0;
> +	return snd_pcm_hw_constraint_eld(substream->runtime,
> +				dai_map->pin->eld.eld_buffer);


...here.
Takashi Iwai Jan. 8, 2016, 2:48 p.m. UTC | #2 
On Fri, 08 Jan 2016 15:09:33 +0100,
Mark Brown wrote:
> 
> On Wed, Dec 09, 2015 at 09:46:12PM +0530, Subhransu S. Prusty wrote:
> 
> > +static int hdac_hdmi_eld_limit_formats(struct snd_pcm_runtime *runtime,
> > +						void *eld)
> > +{
> > +	u64 formats = SNDRV_PCM_FMTBIT_S16;
> > +	int i;
> > +	const u8 *sad, *eld_buf = eld;
> > +
> > +	sad = drm_eld_sad(eld_buf);
> > +	if (!sad)
> > +		goto format_constraint;
> > +
> > +	for (i = drm_eld_sad_count(eld_buf); i > 0; i--, sad += 3) {
> > +		if (sad_format(sad) == 1) { /* AUDIO_CODING_TYPE_LPCM */
> 
> 	switch (sad_format(sad))  {
> 	case AUDIO_CODING_TYPE_LPCM:
> 
> > +
> > +			/* 20 bit and 24 bit */
> > +			if (sad_sample_bits_lpcm(sad) & 0x6)
> > +				formats |= SNDRV_PCM_FMTBIT_S32;
> > +		}
> > +	}
> 
> I really don't have a clear idea what the above is supposed to do or why
> this is specific to HDAC and not handled...
> 
> > -	return 0;
> > +	return snd_pcm_hw_constraint_eld(substream->runtime,
> > +				dai_map->pin->eld.eld_buffer);
> 
> 
> ...here.


The PCM format cannot be determined uniquely as it depends on the
container type and endianness.  So this can't be implemented in a
generic helper.


Takashi
Vinod Koul Jan. 14, 2016, 8:51 a.m. UTC | #3 
On Fri, Jan 08, 2016 at 03:48:03PM +0100, Takashi Iwai wrote:
> On Fri, 08 Jan 2016 15:09:33 +0100,
> Mark Brown wrote:
> > 
> > On Wed, Dec 09, 2015 at 09:46:12PM +0530, Subhransu S. Prusty wrote:
> > 
> > > +static int hdac_hdmi_eld_limit_formats(struct snd_pcm_runtime *runtime,
> > > +						void *eld)
> > > +{
> > > +	u64 formats = SNDRV_PCM_FMTBIT_S16;
> > > +	int i;
> > > +	const u8 *sad, *eld_buf = eld;
> > > +
> > > +	sad = drm_eld_sad(eld_buf);
> > > +	if (!sad)
> > > +		goto format_constraint;
> > > +
> > > +	for (i = drm_eld_sad_count(eld_buf); i > 0; i--, sad += 3) {
> > > +		if (sad_format(sad) == 1) { /* AUDIO_CODING_TYPE_LPCM */
> > 
> > 	switch (sad_format(sad))  {
> > 	case AUDIO_CODING_TYPE_LPCM:
> > 
> > > +
> > > +			/* 20 bit and 24 bit */
> > > +			if (sad_sample_bits_lpcm(sad) & 0x6)
> > > +				formats |= SNDRV_PCM_FMTBIT_S32;
> > > +		}
> > > +	}
> > 
> > I really don't have a clear idea what the above is supposed to do or why
> > this is specific to HDAC and not handled...
> > 
> > > -	return 0;
> > > +	return snd_pcm_hw_constraint_eld(substream->runtime,
> > > +				dai_map->pin->eld.eld_buffer);
> > 
> > 
> > ...here.
> 
> 
> The PCM format cannot be determined uniquely as it depends on the
> container type and endianness.  So this can't be implemented in a
> generic helper.

Hi Mark,

Initially we did start out by adding these in common code (v1) and per
Takashi's feedback we moved this back to driver.

Please let us know if you are fine with approach

Thanks
diff mbox

Patch

diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
index c0c5c8e..f805cb6 100644
--- a/sound/soc/codecs/Kconfig
+++ b/sound/soc/codecs/Kconfig
@@ -486,6 +486,7 @@  config SND_SOC_GTM601
 config SND_SOC_HDAC_HDMI
 	tristate
 	select SND_HDA_EXT_CORE
+	select SND_PCM_ELD
 	select HDMI
 
 config SND_SOC_ICS43432
diff --git a/sound/soc/codecs/hdac_hdmi.c b/sound/soc/codecs/hdac_hdmi.c
index 016aa24..3e2a12e 100644
--- a/sound/soc/codecs/hdac_hdmi.c
+++ b/sound/soc/codecs/hdac_hdmi.c
@@ -22,10 +22,12 @@ 
 #include <linux/module.h>
 #include <linux/pm_runtime.h>
 #include <linux/hdmi.h>
+#include <drm/drm_edid.h>
 #include <sound/pcm_params.h>
 #include <sound/soc.h>
 #include <sound/hdaudio_ext.h>
 #include <sound/hda_i915.h>
+#include <sound/pcm_drm_eld.h>
 #include "../../hda/local.h"
 
 #define AMP_OUT_MUTE		0xb080
@@ -90,6 +92,42 @@  static inline struct hdac_ext_device *to_hda_ext_device(struct device *dev)
 	return container_of(hdac, struct hdac_ext_device, hdac);
 }
 
+static unsigned int sad_format(const u8 *sad)
+{
+	return ((sad[0] >> 0x3) & 0x1f);
+}
+
+static unsigned int sad_sample_bits_lpcm(const u8 *sad)
+{
+	return (sad[2] & 7);
+}
+
+static int hdac_hdmi_eld_limit_formats(struct snd_pcm_runtime *runtime,
+						void *eld)
+{
+	u64 formats = SNDRV_PCM_FMTBIT_S16;
+	int i;
+	const u8 *sad, *eld_buf = eld;
+
+	sad = drm_eld_sad(eld_buf);
+	if (!sad)
+		goto format_constraint;
+
+	for (i = drm_eld_sad_count(eld_buf); i > 0; i--, sad += 3) {
+		if (sad_format(sad) == 1) { /* AUDIO_CODING_TYPE_LPCM */
+
+			/* 20 bit and 24 bit */
+			if (sad_sample_bits_lpcm(sad) & 0x6)
+				formats |= SNDRV_PCM_FMTBIT_S32;
+		}
+	}
+
+format_constraint:
+	return snd_pcm_hw_constraint_mask64(runtime, SNDRV_PCM_HW_PARAM_FORMAT,
+				formats);
+
+}
+
  /* HDMI Eld routines */
 static unsigned int hdac_hdmi_get_eld_data(struct hdac_device *codec,
 				hda_nid_t nid, int byte_index)
@@ -335,6 +373,7 @@  static int hdac_hdmi_pcm_open(struct snd_pcm_substream *substream,
 	struct hdac_ext_device *hdac = snd_soc_dai_get_drvdata(dai);
 	struct hdac_hdmi_priv *hdmi = hdac->private_data;
 	struct hdac_hdmi_dai_pin_map *dai_map;
+	int ret;
 
 	if (dai->id > 0) {
 		dev_err(&hdac->hdac.dev, "Only one dai supported as of now\n");
@@ -356,10 +395,13 @@  static int hdac_hdmi_pcm_open(struct snd_pcm_substream *substream,
 	snd_hdac_codec_write(&hdac->hdac, dai_map->pin->nid, 0,
 			AC_VERB_SET_AMP_GAIN_MUTE, AMP_OUT_UNMUTE);
 
-	snd_pcm_hw_constraint_step(substream->runtime, 0,
-				   SNDRV_PCM_HW_PARAM_CHANNELS, 2);
+	ret = hdac_hdmi_eld_limit_formats(substream->runtime,
+				dai_map->pin->eld.eld_buffer);
+	if (ret < 0)
+		return ret;
 
-	return 0;
+	return snd_pcm_hw_constraint_eld(substream->runtime,
+				dai_map->pin->eld.eld_buffer);
 }
 
 static void hdac_hdmi_pcm_close(struct snd_pcm_substream *substream,