-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Wednesday, November 18, 2015 7:31 PM To: libin.yang@linux.intel.com Cc: alsa-devel@alsa-project.org; mengdong.lin@linux.intel.com; Yang, Libin Subject: Re: [alsa-devel] [RFC PATCH 2/2] ALSA: hda - hdmi dynamically bind PCM to pin when monitor hotplug
On Wed, 18 Nov 2015 09:46:59 +0100, libin.yang@linux.intel.com wrote:
From: Libin Yang libin.yang@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@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 for review. I will split the patch into several ones.
Regards, Libin
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;@@ -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++;@@ -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