Re: [alsa-devel] [PATCH 1/3] ALSA: hda - Create common chmap object

On Mon, Feb 22, 2016 at 10:32:33AM +0100, Takashi Iwai wrote:

On Mon, 22 Feb 2016 09:00:38 +0100, Subhransu S. Prusty wrote:

...

Common chmap object represents multichannel capablity and contains chmap ops. Legacy driver is udpated to use this and later ASoC hdac hdmi driver will also use.

In the mixer control callback handlers, information specific to driver are accessed through ops. Also during chmap control registration, use the common hdmi_chmap object instead of hda_codec.

Signed-off-by: Subhransu S. Prusty subhransu.s.prusty@intel.com Signed-off-by: Vinod Koul vinod.koul@intel.com

To make review easier, could you split this patch into two, one for just moving the hdmi_chmap out of spec, and another is the addition of new ops, get_chmap, set_chmap, etc.

Sure, will do it.

thanks,

Takashi

...

---

include/sound/hdmi_chmap.h | 39 ++++++++++++ sound/pci/hda/patch_hdmi.c | 145 ++++++++++++++++++++++++++++----------------- 2 files changed, 131 insertions(+), 53 deletions(-) create mode 100644 include/sound/hdmi_chmap.h

diff --git a/include/sound/hdmi_chmap.h b/include/sound/hdmi_chmap.h new file mode 100644 index 0000000..e5ea02a --- /dev/null +++ b/include/sound/hdmi_chmap.h @@ -0,0 +1,39 @@ +/*

• • For multichannel support
• */

+#ifndef __SOUND_HDMI_CHMAP_H +#define __SOUND_HDMI_CHMAP_H

+#include <sound/hdaudio.h>

+struct cea_channel_speaker_allocation;

+struct hdmi_chmap_ops {

• /*

• * Helpers for producing the channel map TLVs. These can be overridden
  

• * for devices that have non-standard mapping requirements.
  

• */
  

• int (*chmap_cea_alloc_validate_get_type)

• (struct cea_channel_speaker_allocation *cap, int channels);
  

• void (*cea_alloc_to_tlv_chmap)

• (struct cea_channel_speaker_allocation *cap,
  

• unsigned int *chmap, int channels);
  

• /* check that the user-given chmap is supported */
• int (*chmap_validate)(int ca, int channels, unsigned char *chmap);
• void (*get_chmap)(struct hdac_device *hdac, int pcm_idx,

• 			unsigned char *chmap);
  

• void (*set_chmap)(struct hdac_device *hdac, int pcm_idx,

• 	unsigned char *chmap, int prepared);
  

• bool (*is_monitor_connected)(struct hdac_device *hdac, int pcm_idx);

+};

+struct hdmi_chmap {

• unsigned int channels_max; /* max over all cvts */
• struct hdmi_chmap_ops ops;
• struct hdac_device *hdac;

+};

+#endif /* __SOUND_HDMI_CHMAP_H */ diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index f4443b5..d73698c 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -37,8 +37,10 @@ #include <sound/jack.h> #include <sound/asoundef.h> #include <sound/tlv.h>

#include <sound/hdaudio.h> #include <sound/hda_i915.h> +#include <sound/hdmi_chmap.h> #include "hda_codec.h" #include "hda_local.h" #include "hda_jack.h" @@ -101,7 +103,6 @@ struct hdmi_spec_per_pin { };

struct cea_channel_speaker_allocation;

/* operations used by generic code that can be overridden by patches */ struct hdmi_ops { int (*pin_get_eld)(struct hda_codec *codec, hda_nid_t pin_nid, @@ -122,15 +123,6 @@ struct hdmi_ops { int (*setup_stream)(struct hda_codec *codec, hda_nid_t cvt_nid, hda_nid_t pin_nid, u32 stream_tag, int format);

• /* Helpers for producing the channel map TLVs. These can be overridden

• * for devices that have non-standard mapping requirements. */
  

• int (*chmap_cea_alloc_validate_get_type)(struct cea_channel_speaker_allocation *cap,

• 				 int channels);
  

• void (*cea_alloc_to_tlv_chmap)(struct cea_channel_speaker_allocation *cap,

• 		       unsigned int *chmap, int channels);
  

• /* check that the user-given chmap is supported */
• int (*chmap_validate)(int ca, int channels, unsigned char *chmap);

};

struct hdmi_pcm { @@ -155,7 +147,6 @@ struct hdmi_spec { * bit 1 means the second playback PCM, and so on. */ unsigned long pcm_in_use;

• unsigned int channels_max; /* max over all cvts */

  struct hdmi_eld temp_eld; struct hdmi_ops ops;

@@ -171,6 +162,8 @@ struct hdmi_spec { /* i915/powerwell (Haswell+/Valleyview+) specific */ struct i915_audio_component_audio_ops i915_audio_ops; bool i915_bound; /* was i915 bound in this driver? */

• struct hdmi_chmap chmap;

};

#ifdef CONFIG_SND_HDA_I915 @@ -2096,8 +2089,8 @@ static int hdmi_add_cvt(struct hda_codec *codec, hda_nid_t cvt_nid) per_cvt->channels_min = 2; if (chans <= 16) { per_cvt->channels_max = chans;

• if (chans > spec->channels_max)
  

• 	spec->channels_max = chans;
  

• if (chans > spec->chmap.channels_max)
  

• 	spec->chmap.channels_max = chans;
  

  }

  err = snd_hda_query_supported_pcm(codec, cvt_nid,

@@ -2321,10 +2314,10 @@ static int hdmi_chmap_ctl_info(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_info *uinfo) { struct snd_pcm_chmap *info = snd_kcontrol_chip(kcontrol);

• struct hda_codec *codec = info->private_data;
• struct hdmi_spec *spec = codec->spec;

• struct hdmi_chmap *chmap = info->private_data;
• uinfo->type = SNDRV_CTL_ELEM_TYPE_INTEGER;

• uinfo->count = spec->channels_max;

• uinfo->count = chmap->channels_max; uinfo->value.integer.min = 0; uinfo->value.integer.max = SNDRV_CHMAP_LAST; return 0;

@@ -2358,12 +2351,49 @@ static void hdmi_cea_alloc_to_tlv_chmap(struct cea_channel_speaker_allocation *c WARN_ON(count != channels); }

+static void hdmi_get_chmap(struct hdac_device *hdac, int pcm_idx,

• 			unsigned char *chmap)
  

+{

• struct hda_codec *codec = container_of(hdac, struct hda_codec, core);
• struct hdmi_spec *spec = codec->spec;
• struct hdmi_spec_per_pin *per_pin = pcm_idx_to_pin(spec, pcm_idx);
• /* chmap is already set to 0 in caller */
• if (!per_pin)

• return;
  

• memcpy(chmap, per_pin->chmap, ARRAY_SIZE(per_pin->chmap));

+}

+static void hdmi_set_chmap(struct hdac_device *hdac, int pcm_idx,

• 		unsigned char *chmap, int prepared)
  

+{

• struct hda_codec *codec = container_of(hdac, struct hda_codec, core);
• struct hdmi_spec *spec = codec->spec;
• struct hdmi_spec_per_pin *per_pin = pcm_idx_to_pin(spec, pcm_idx);
• mutex_lock(&per_pin->lock);
• per_pin->chmap_set = true;
• memcpy(per_pin->chmap, chmap, ARRAY_SIZE(per_pin->chmap));
• if (prepared)

• hdmi_setup_audio_infoframe(codec, per_pin, per_pin->non_pcm);
  

• mutex_unlock(&per_pin->lock);

+}

+static bool is_hdmi_monitor_connected(struct hdac_device *hdac, int pcm_idx) +{

• struct hda_codec *codec = container_of(hdac, struct hda_codec, core);
• struct hdmi_spec *spec = codec->spec;
• struct hdmi_spec_per_pin *per_pin = pcm_idx_to_pin(spec, pcm_idx);
• return per_pin ? true:false;

+}

static int hdmi_chmap_ctl_tlv(struct snd_kcontrol *kcontrol, int op_flag, unsigned int size, unsigned int __user *tlv) { struct snd_pcm_chmap *info = snd_kcontrol_chip(kcontrol);

• struct hda_codec *codec = info->private_data;
• struct hdmi_spec *spec = codec->spec;

• struct hdmi_chmap *chmap = info->private_data; unsigned int __user *dst; int chs, count = 0;

@@ -2373,13 +2403,14 @@ static int hdmi_chmap_ctl_tlv(struct snd_kcontrol *kcontrol, int op_flag, return -EFAULT; size -= 8; dst = tlv + 2;

• for (chs = 2; chs <= spec->channels_max; chs++) {

• for (chs = 2; chs <= chmap->channels_max; chs++) { int i; struct cea_channel_speaker_allocation *cap; cap = channel_allocations; for (i = 0; i < ARRAY_SIZE(channel_allocations); i++, cap++) { int chs_bytes = chs * 4;

• 	int type = spec->ops.chmap_cea_alloc_validate_get_type(cap, chs);
  

• 	int type = chmap->ops.chmap_cea_alloc_validate_get_type(
  

• 						cap, chs);
  unsigned int tlv_chmap[8];
  
  if (type < 0)
  

@@ -2396,7 +2427,7 @@ static int hdmi_chmap_ctl_tlv(struct snd_kcontrol *kcontrol, int op_flag, return -ENOMEM; size -= chs_bytes; count += chs_bytes;

• 	spec->ops.cea_alloc_to_tlv_chmap(cap, tlv_chmap, chs);
  

• 	chmap->ops.cea_alloc_to_tlv_chmap(cap, tlv_chmap, chs);
  if (copy_to_user(dst, tlv_chmap, chs_bytes))
  	return -EFAULT;
  dst += chs;
  

@@ -2411,20 +2442,17 @@ static int hdmi_chmap_ctl_get(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_value *ucontrol) { struct snd_pcm_chmap *info = snd_kcontrol_chip(kcontrol);

• struct hda_codec *codec = info->private_data;
• struct hdmi_spec *spec = codec->spec;

• struct hdmi_chmap *hchmap = info->private_data; int pcm_idx = kcontrol->private_value;

• struct hdmi_spec_per_pin *per_pin = pcm_idx_to_pin(spec, pcm_idx);

• unsigned char chmap[8]; int i;

• if (!per_pin) {

• for (i = 0; i < spec->channels_max; i++)
  

• 	ucontrol->value.integer.value[i] = 0;
  

• return 0;
  

• }

• memset(chmap, 0, sizeof(chmap));
• hchmap->ops.get_chmap(hchmap->hdac, pcm_idx, chmap);
• for (i = 0; i < sizeof(chmap); i++)

• ucontrol->value.integer.value[i] = chmap[i];
  
  

• for (i = 0; i < ARRAY_SIZE(per_pin->chmap); i++)

• ucontrol->value.integer.value[i] = per_pin->chmap[i];
  

  return 0;

}

@@ -2432,19 +2460,17 @@ static int hdmi_chmap_ctl_put(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_value *ucontrol) { struct snd_pcm_chmap *info = snd_kcontrol_chip(kcontrol);

• struct hda_codec *codec = info->private_data;
• struct hdmi_spec *spec = codec->spec;

• struct hdmi_chmap *hchmap = info->private_data; int pcm_idx = kcontrol->private_value;

• struct hdmi_spec_per_pin *per_pin = pcm_idx_to_pin(spec, pcm_idx); unsigned int ctl_idx; struct snd_pcm_substream *substream;
• unsigned char chmap[8];

• unsigned char chmap[8], per_pin_chmap[8]; int i, err, ca, prepared = 0;

  /* No monitor is connected in dyn_pcm_assign.

  • It's invalid to setup the chmap

  */

• if (!per_pin)

• if (!hchmap->ops.is_monitor_connected(hchmap->hdac, pcm_idx)) return 0;

  ctl_idx = snd_ctl_get_ioffidx(kcontrol, &ucontrol->id);

@@ -2464,22 +2490,20 @@ static int hdmi_chmap_ctl_put(struct snd_kcontrol *kcontrol, memset(chmap, 0, sizeof(chmap)); for (i = 0; i < ARRAY_SIZE(chmap); i++) chmap[i] = ucontrol->value.integer.value[i];

• if (!memcmp(chmap, per_pin->chmap, sizeof(chmap)))

• hchmap->ops.get_chmap(hchmap->hdac, pcm_idx, per_pin_chmap);
• if (!memcmp(chmap, per_pin_chmap, sizeof(chmap))) return 0; ca = hdmi_manual_channel_allocation(ARRAY_SIZE(chmap), chmap); if (ca < 0) return -EINVAL;

• if (spec->ops.chmap_validate) {

• err = spec->ops.chmap_validate(ca, ARRAY_SIZE(chmap), chmap);
  

• if (hchmap->ops.chmap_validate) {

• err = hchmap->ops.chmap_validate(ca, ARRAY_SIZE(chmap), chmap);
  

  if (err) return err; }

• mutex_lock(&per_pin->lock);
• per_pin->chmap_set = true;
• memcpy(per_pin->chmap, chmap, sizeof(chmap));
• if (prepared)

• hdmi_setup_audio_infoframe(codec, per_pin, per_pin->non_pcm);
  

• mutex_unlock(&per_pin->lock);

• hchmap->ops.set_chmap(hchmap->hdac, pcm_idx, chmap, prepared);

  return 0;

} @@ -2638,7 +2662,7 @@ static int generic_hdmi_build_controls(struct hda_codec *codec) if (err < 0) return err; /* override handlers */

• chmap->private_data = codec;
  

• chmap->private_data = &spec->chmap;
  

  kctl = chmap->kctl; for (i = 0; i < kctl->count; i++) kctl->vd[i].access |= SNDRV_CTL_ELEM_ACCESS_WRITE;

@@ -2762,11 +2786,16 @@ static const struct hdmi_ops generic_standard_hdmi_ops = { .pin_setup_infoframe = hdmi_pin_setup_infoframe, .pin_hbr_setup = hdmi_pin_hbr_setup, .setup_stream = hdmi_setup_stream, +};

+static const struct hdmi_chmap_ops chmap_ops = { .chmap_cea_alloc_validate_get_type = hdmi_chmap_cea_alloc_validate_get_type, .cea_alloc_to_tlv_chmap = hdmi_cea_alloc_to_tlv_chmap,

• .get_chmap = hdmi_get_chmap,
• .set_chmap = hdmi_set_chmap,
• .is_monitor_connected = is_hdmi_monitor_connected,

};

static void intel_haswell_fixup_connect_list(struct hda_codec *codec, hda_nid_t nid) { @@ -2868,6 +2897,8 @@ static int patch_generic_hdmi(struct hda_codec *codec)

spec->ops = generic_standard_hdmi_ops; mutex_init(&spec->pcm_lock);

• spec->chmap.ops = chmap_ops;
• spec->chmap.hdac = &codec->core; codec->spec = spec; hdmi_array_init(spec, 4);

@@ -3483,9 +3514,12 @@ static int patch_nvhdmi(struct hda_codec *codec) spec = codec->spec; spec->dyn_pin_out = true;

• spec->ops.chmap_cea_alloc_validate_get_type =

• spec->chmap.ops.chmap_cea_alloc_validate_get_type = nvhdmi_chmap_cea_alloc_validate_get_type;

• spec->ops.chmap_validate = nvhdmi_chmap_validate;

• spec->chmap.ops.chmap_validate = nvhdmi_chmap_validate;

• spec->chmap.ops.get_chmap = hdmi_get_chmap;

• spec->chmap.ops.set_chmap = hdmi_set_chmap;

• spec->chmap.ops.is_monitor_connected = is_hdmi_monitor_connected;

  return 0;

} @@ -3992,10 +4026,15 @@ static int patch_atihdmi(struct hda_codec *codec)

if (!has_amd_full_remap_support(codec)) { /* override to ATI/AMD-specific versions with pairwise mapping */

• spec->ops.chmap_cea_alloc_validate_get_type =
  

• spec->chmap.ops.chmap_cea_alloc_validate_get_type =
  atihdmi_paired_chmap_cea_alloc_validate_get_type;
  

• spec->ops.cea_alloc_to_tlv_chmap = atihdmi_paired_cea_alloc_to_tlv_chmap;
  

• spec->ops.chmap_validate = atihdmi_paired_chmap_validate;
  

• spec->chmap.ops.cea_alloc_to_tlv_chmap =
  

• 		atihdmi_paired_cea_alloc_to_tlv_chmap;
  

• spec->chmap.ops.chmap_validate = atihdmi_paired_chmap_validate;
  

• spec->chmap.ops.get_chmap = hdmi_get_chmap;
  

• spec->chmap.ops.set_chmap = hdmi_set_chmap;
  

• spec->chmap.ops.is_monitor_connected =
  

• 		is_hdmi_monitor_connected;
  

  }

  /* ATI/AMD converters do not advertise all of their capabilities */

@@ -4007,7 +4046,7 @@ static int patch_atihdmi(struct hda_codec *codec) per_cvt->maxbps = max(per_cvt->maxbps, 24u); }

• spec->channels_max = max(spec->channels_max, 8u);

• spec->chmap.channels_max = max(spec->chmap.channels_max, 8u);

  return 0;

}

1.9.1

--