[alsa-devel] [RFC PATCH 2/2] ALSA: hda - hdmi dynamically bind PCM to pin when monitor hotplug
Takashi Iwai
tiwai at suse.de
Wed Nov 18 12:31:26 CET 2015
On Wed, 18 Nov 2015 09:46:59 +0100,
libin.yang at linux.intel.com wrote:
>
> From: Libin Yang <libin.yang at linux.intel.com>
>
> Dynamically bind/unbind the PCM to pin when HDMI/DP monitor hotplug.
>
> When monitor is connected, find a proper PCM for the monitor.
> If PCM is already open, set the correct configuration for the pin.
>
> When monitor is disconnected, unbind the PCM from the pin if
> PCM is not open. Otherwise unbind the PCM when pcm closes.
Well, there are still too many logical changes in a single patch
although the code size itself isn't too big.
Let's see:
>
> Signed-off-by: Libin Yang <libin.yang at linux.intel.com>
> ---
> sound/pci/hda/hda_codec.h | 1 +
> sound/pci/hda/patch_hdmi.c | 197 ++++++++++++++++++++++++++++++++++++++++++---
> 2 files changed, 188 insertions(+), 10 deletions(-)
>
> diff --git a/sound/pci/hda/hda_codec.h b/sound/pci/hda/hda_codec.h
> index 373fcad..ee97401 100644
> --- a/sound/pci/hda/hda_codec.h
> +++ b/sound/pci/hda/hda_codec.h
> @@ -167,6 +167,7 @@ enum {
> /* for PCM creation */
> struct hda_pcm {
> char *name;
> + bool in_use;
Now here is a new flag...
> struct hda_pcm_stream stream[2];
> unsigned int pcm_type; /* HDA_PCM_TYPE_XXX */
> int device; /* device number to assign */
> diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
> index 12fc506..77d6701 100644
> --- a/sound/pci/hda/patch_hdmi.c
> +++ b/sound/pci/hda/patch_hdmi.c
> @@ -72,6 +72,8 @@ struct hdmi_spec_per_cvt {
>
> struct hdmi_spec_per_pin {
> hda_nid_t pin_nid;
> + /* pin idx, different device entries on the same pin use the same idx */
> + int pin_nid_idx;
And a new field...
> int num_mux_nids;
> hda_nid_t mux_nids[HDA_MAX_CONNECTIONS];
> int mux_idx;
> @@ -83,6 +85,7 @@ struct hdmi_spec_per_pin {
> struct delayed_work work;
> struct snd_kcontrol *eld_ctl;
> struct hda_pcm *pcm; /* pointer to spec->pcm_rec[n] dynamically*/
> + int pcm_idx; /* which pcm is attached. -1 means no pcm is attached */
Yet a new one...
> int repoll_count;
> bool setup; /* the stream has been set up by prepare callback */
> int channels; /* current number of channels */
> @@ -136,6 +139,8 @@ struct hdmi_spec {
> struct snd_array pins; /* struct hdmi_spec_per_pin */
> struct hda_pcm *pcm_rec[16];
> struct mutex pcm_lock;
> + unsigned long pcm_bitmap;
> + int pcm_used; /* counter of pcm_rec[] */
Two new fields. This already looks suspicious. Basically these
additions imply:
- the patch introduces a new usage flag per pin,
- the patch introduces a new index number for MST dev,
- the patch introduces a new index number for a PCM per pin,
- the patch introduces the new concept of PCM numbers, and
- the patch introduces some bitmap management.
This makes hard to understand what this patch does; simply because it
does so many different things.
The whole changes look reasonable to me in a quick glance, so we can
go in that direction. But let's organize the patches better.
thanks,
Takashi
> unsigned int channels_max; /* max over all cvts */
>
> struct hdmi_eld temp_eld;
> @@ -376,6 +381,20 @@ static int pin_nid_to_pin_index(struct hda_codec *codec, hda_nid_t pin_nid)
> return -EINVAL;
> }
>
> +static int hinfo_to_pcm_index(struct hda_codec *codec,
> + struct hda_pcm_stream *hinfo)
> +{
> + struct hdmi_spec *spec = codec->spec;
> + int pcm_idx;
> +
> + for (pcm_idx = 0; pcm_idx < spec->pcm_used; pcm_idx++)
> + if (get_pcm_rec(spec, pcm_idx)->stream == hinfo)
> + return pcm_idx;
> +
> + codec_warn(codec, "HDMI: hinfo %p not registered\n", hinfo);
> + return -EINVAL;
> +}
> +
> static int hinfo_to_pin_index(struct hda_codec *codec,
> struct hda_pcm_stream *hinfo)
> {
> @@ -1486,10 +1505,14 @@ static int hdmi_pcm_open_no_pin(struct hda_pcm_stream *hinfo,
> {
> struct hdmi_spec *spec = codec->spec;
> struct snd_pcm_runtime *runtime = substream->runtime;
> - int cvt_idx, mux_idx;
> + int cvt_idx, pcm_idx, mux_idx;
> struct hdmi_spec_per_cvt *per_cvt = NULL;
> int err;
>
> + pcm_idx = hinfo_to_pcm_index(codec, hinfo);
> + if (pcm_idx < 0)
> + return -EINVAL;
> +
> err = hdmi_find_available_cvt(codec, &cvt_idx);
> if (err)
> return err;
> @@ -1497,11 +1520,11 @@ static int hdmi_pcm_open_no_pin(struct hda_pcm_stream *hinfo,
> per_cvt = get_cvt(spec, cvt_idx);
> per_cvt->assigned = 1;
> hinfo->nid = per_cvt->cvt_nid;
> -
> mux_idx = intel_cvt_id_to_mux_idx(spec, per_cvt->cvt_nid);
> /* unshare converter from all pins */
> intel_not_share_assigned_cvt(codec, 0, mux_idx);
>
> + spec->pcm_rec[pcm_idx]->in_use = true;
> /* todo: setup spdif ctls assign */
>
> /* Initially set the converter's capabilities */
> @@ -1531,13 +1554,17 @@ static int hdmi_pcm_open(struct hda_pcm_stream *hinfo,
> {
> struct hdmi_spec *spec = codec->spec;
> struct snd_pcm_runtime *runtime = substream->runtime;
> - int pin_idx, cvt_idx, mux_idx = 0;
> + int pin_idx, cvt_idx, pcm_idx, mux_idx = 0;
> struct hdmi_spec_per_pin *per_pin;
> struct hdmi_eld *eld;
> struct hdmi_spec_per_cvt *per_cvt = NULL;
> int err;
>
> /* Validate hinfo */
> + pcm_idx = hinfo_to_pcm_index(codec, hinfo);
> + if (pcm_idx < 0)
> + return -EINVAL;
> +
> mutex_lock(&spec->pcm_lock);
> pin_idx = hinfo_to_pin_index(codec, hinfo);
> if (!spec->dyn_pcm_assign) {
> @@ -1566,7 +1593,7 @@ static int hdmi_pcm_open(struct hda_pcm_stream *hinfo,
> /* Claim converter */
> per_cvt->assigned = 1;
>
> -
> + spec->pcm_rec[pcm_idx]->in_use = true;
> per_pin = get_pin(spec, pin_idx);
> per_pin->cvt_nid = per_cvt->cvt_nid;
> hinfo->nid = per_cvt->cvt_nid;
> @@ -1579,7 +1606,7 @@ static int hdmi_pcm_open(struct hda_pcm_stream *hinfo,
> if (is_haswell_plus(codec) || is_valleyview_plus(codec))
> intel_not_share_assigned_cvt(codec, per_pin->pin_nid, mux_idx);
>
> - snd_hda_spdif_ctls_assign(codec, pin_idx, per_cvt->cvt_nid);
> + snd_hda_spdif_ctls_assign(codec, pcm_idx, per_cvt->cvt_nid);
>
> /* Initially set the converter's capabilities */
> hinfo->channels_min = per_cvt->channels_min;
> @@ -1596,7 +1623,7 @@ static int hdmi_pcm_open(struct hda_pcm_stream *hinfo,
> !hinfo->rates || !hinfo->formats) {
> per_cvt->assigned = 0;
> hinfo->nid = 0;
> - snd_hda_spdif_ctls_unassign(codec, pin_idx);
> + snd_hda_spdif_ctls_unassign(codec, pcm_idx);
> mutex_unlock(&spec->pcm_lock);
> return -ENODEV;
> }
> @@ -1637,6 +1664,135 @@ static int hdmi_read_pin_conn(struct hda_codec *codec, int pin_idx)
> return 0;
> }
>
> +static int get_preferred_pcm_slot(struct hdmi_spec *spec,
> + struct hdmi_spec_per_pin *per_pin)
> +{
> + if ((spec->pcm_bitmap & (1 << per_pin->pin_nid_idx)) == 0)
> + return per_pin->pin_nid_idx;
> + return -1;
> +}
> +
> +static int hdmi_find_pcm_slot(struct hdmi_spec *spec,
> + struct hdmi_spec_per_pin *per_pin)
> +{
> + int slot;
> + int i;
> +
> + /* try the prefer PCM */
> + slot = get_preferred_pcm_slot(spec, per_pin);
> + if (slot != -1)
> + return slot;
> +
> + /* have a second try */
> + for (i = spec->num_pins; i < spec->pcm_used; i++) {
> + if ((spec->pcm_bitmap & (1 << i)) == 0)
> + return i;
> + }
> +
> + /* the last try */
> + for (i = 0; i < spec->pcm_used; i++) {
> + if ((spec->pcm_bitmap & (1 << i)) == 0)
> + return i;
> + }
> + return -ENODEV;
> +}
> +
> +static void hdmi_attach_hda_pcm(struct hdmi_spec *spec,
> + struct hdmi_spec_per_pin *per_pin)
> +{
> + int idx;
> +
> + /* pcm already be attached to the pin */
> + if (per_pin->pcm)
> + return;
> + idx = hdmi_find_pcm_slot(spec, per_pin);
> + if (idx == -ENODEV)
> + return;
> + per_pin->pcm_idx = idx;
> + per_pin->pcm = spec->pcm_rec[idx];
> + set_bit(idx, &spec->pcm_bitmap);
> +}
> +
> +static void hdmi_detach_hda_pcm(struct hdmi_spec *spec,
> + struct hdmi_spec_per_pin *per_pin)
> +{
> + int idx;
> +
> + /* pcm already be detached from the pin */
> + if (!per_pin->pcm)
> + return;
> + idx = per_pin->pcm_idx;
> + per_pin->pcm_idx = -1;
> + per_pin->pcm = NULL;
> + if (idx >= 0 && idx < spec->pcm_used)
> + clear_bit(idx, &spec->pcm_bitmap);
> +}
> +
> +static int hdmi_get_pin_cvt_mux(struct hdmi_spec *spec,
> + struct hdmi_spec_per_pin *per_pin, hda_nid_t cvt_nid)
> +{
> + int mux_idx;
> +
> + for (mux_idx = 0; mux_idx < per_pin->num_mux_nids; mux_idx++)
> + if (per_pin->mux_nids[mux_idx] == cvt_nid)
> + break;
> + return mux_idx;
> +}
> +
> +static bool check_non_pcm_per_cvt(struct hda_codec *codec, hda_nid_t cvt_nid);
> +static void hdmi_pcm_setup_pin(struct hdmi_spec *spec,
> + struct hdmi_spec_per_pin *per_pin)
> +{
> + struct hda_codec *codec = per_pin->codec;
> + struct hda_pcm *pcm;
> + struct hda_pcm_stream *hinfo;
> + struct snd_pcm_substream *substream;
> +
> + int mux_idx;
> + bool non_pcm;
> +
> + if (per_pin->pcm_idx >= 0 && per_pin->pcm_idx < spec->pcm_used)
> + pcm = spec->pcm_rec[per_pin->pcm_idx];
> + else
> + return;
> + if (!pcm->in_use)
> + return;
> +
> + /* hdmi audio only uses playback and one substream */
> + hinfo = pcm->stream;
> + substream = pcm->pcm->streams[0].substream;
> +
> + per_pin->cvt_nid = hinfo->nid;
> +
> + mux_idx = hdmi_get_pin_cvt_mux(spec, per_pin, hinfo->nid);
> + if (mux_idx < per_pin->num_mux_nids)
> + snd_hda_codec_write_cache(codec, per_pin->pin_nid, 0,
> + AC_VERB_SET_CONNECT_SEL,
> + mux_idx);
> + snd_hda_spdif_ctls_assign(codec, per_pin->pcm_idx, hinfo->nid);
> +
> + non_pcm = check_non_pcm_per_cvt(codec, hinfo->nid);
> + if (substream->runtime)
> + per_pin->channels = substream->runtime->channels;
> + per_pin->setup = true;
> + per_pin->mux_idx = mux_idx;
> +
> + hdmi_setup_audio_infoframe(codec, per_pin, non_pcm);
> +}
> +
> +static void hdmi_pcm_reset_pin(struct hdmi_spec *spec,
> + struct hdmi_spec_per_pin *per_pin)
> +{
> + if (per_pin->pcm_idx >= 0 && per_pin->pcm_idx < spec->pcm_used)
> + snd_hda_spdif_ctls_unassign(per_pin->codec, per_pin->pcm_idx);
> +
> + per_pin->chmap_set = false;
> + memset(per_pin->chmap, 0, sizeof(per_pin->chmap));
> +
> + per_pin->setup = false;
> + per_pin->channels = 0;
> +}
> +
> static bool hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, int repoll)
> {
> struct hda_jack_tbl *jack;
> @@ -1661,6 +1817,7 @@ static bool hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, int repoll)
> snd_hda_power_up_pm(codec);
> present = snd_hda_pin_sense(codec, pin_nid);
>
> + mutex_lock(&spec->pcm_lock);
> mutex_lock(&per_pin->lock);
> pin_eld->monitor_present = !!(present & AC_PINSENSE_PRESENCE);
> if (pin_eld->monitor_present)
> @@ -1694,6 +1851,16 @@ static bool hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, int repoll)
> }
> }
>
> + if (spec->dyn_pcm_assign) {
> + if (eld->eld_valid) {
> + hdmi_attach_hda_pcm(spec, per_pin);
> + hdmi_pcm_setup_pin(spec, per_pin);
> + } else {
> + hdmi_pcm_reset_pin(spec, per_pin);
> + hdmi_detach_hda_pcm(spec, per_pin);
> + }
> + }
> +
> if (pin_eld->eld_valid != eld->eld_valid)
> eld_changed = true;
>
> @@ -1744,6 +1911,7 @@ static bool hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, int repoll)
> jack->block_report = !ret;
>
> mutex_unlock(&per_pin->lock);
> + mutex_unlock(&spec->pcm_lock);
> snd_hda_power_down_pm(codec);
> return ret;
> }
> @@ -1789,6 +1957,11 @@ static int hdmi_add_pin(struct hda_codec *codec, hda_nid_t pin_nid)
>
> per_pin->pin_nid = pin_nid;
> per_pin->non_pcm = false;
> + if (spec->dyn_pcm_assign)
> + per_pin->pcm_idx = -1;
> + else
> + per_pin->pcm_idx = pin_idx;
> + per_pin->pin_nid_idx = pin_idx;
>
> err = hdmi_read_pin_conn(codec, pin_idx);
> if (err < 0)
> @@ -1994,22 +2167,25 @@ static int hdmi_pcm_close(struct hda_pcm_stream *hinfo,
> struct snd_pcm_substream *substream)
> {
> struct hdmi_spec *spec = codec->spec;
> - int cvt_idx, pin_idx;
> + int cvt_idx, pin_idx, pcm_idx;
> struct hdmi_spec_per_cvt *per_cvt;
> struct hdmi_spec_per_pin *per_pin;
> int pinctl;
>
> if (hinfo->nid) {
> + pcm_idx = hinfo_to_pcm_index(codec, hinfo);
> + if (snd_BUG_ON(pcm_idx < 0))
> + return -EINVAL;
> cvt_idx = cvt_nid_to_cvt_index(codec, hinfo->nid);
> if (snd_BUG_ON(cvt_idx < 0))
> return -EINVAL;
> per_cvt = get_cvt(spec, cvt_idx);
> -
> snd_BUG_ON(!per_cvt->assigned);
> per_cvt->assigned = 0;
> hinfo->nid = 0;
>
> mutex_lock(&spec->pcm_lock);
> + spec->pcm_rec[pcm_idx]->in_use = false;
> pin_idx = hinfo_to_pin_index(codec, hinfo);
> if (spec->dyn_pcm_assign && pin_idx < 0) {
> mutex_unlock(&spec->pcm_lock);
> @@ -2021,7 +2197,6 @@ static int hdmi_pcm_close(struct hda_pcm_stream *hinfo,
> return -EINVAL;
> }
> per_pin = get_pin(spec, pin_idx);
> -
> if (spec->dyn_pin_out) {
> pinctl = snd_hda_codec_read(codec, per_pin->pin_nid, 0,
> AC_VERB_GET_PIN_WIDGET_CONTROL, 0);
> @@ -2030,7 +2205,7 @@ static int hdmi_pcm_close(struct hda_pcm_stream *hinfo,
> pinctl & ~PIN_OUT);
> }
>
> - snd_hda_spdif_ctls_unassign(codec, pin_idx);
> + snd_hda_spdif_ctls_unassign(codec, pcm_idx);
>
> mutex_lock(&per_pin->lock);
> per_pin->chmap_set = false;
> @@ -2228,6 +2403,7 @@ static int generic_hdmi_build_pcms(struct hda_codec *codec)
> per_pin->pcm = info;
> }
> spec->pcm_rec[pin_idx] = info;
> + spec->pcm_used++;
> info->pcm_type = HDA_PCM_TYPE_HDMI;
> info->own_chmap = true;
>
> @@ -2275,6 +2451,7 @@ static int generic_hdmi_build_controls(struct hda_codec *codec)
> HDA_PCM_TYPE_HDMI);
> if (err < 0)
> return err;
> + /* pin number is the same with pcm number so far */
> snd_hda_spdif_ctls_unassign(codec, pin_idx);
>
> /* add control for ELD Bytes */
> --
> 1.9.1
>
More information about the Alsa-devel
mailing list