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@xxxxxxxxx> > Signed-off-by: Vinod Koul <vinod.koul@xxxxxxxxx> 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. 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 > _______________________________________________ Alsa-devel mailing list Alsa-devel@xxxxxxxxxxxxxxxx http://mailman.alsa-project.org/mailman/listinfo/alsa-devel