Re: [RFC PATCH v2 3/3] ALSA - hda: add DP MST support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, 21 Mar 2016 09:37:56 +0100,
libin.yang@xxxxxxxxxxxxxxx wrote:
> 
> From: Libin Yang <libin.yang@xxxxxxxxxxxxxxx>
> 
> This patch adds the DP MST support in hdmi audio driver.
> ---
>  sound/pci/hda/hda_codec.c  |   3 +
>  sound/pci/hda/patch_hdmi.c | 179 ++++++++++++++++++++++++++++++++-------------
>  2 files changed, 133 insertions(+), 49 deletions(-)
> 
> diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c
> index db94a66..d4be769 100644
> --- a/sound/pci/hda/hda_codec.c
> +++ b/sound/pci/hda/hda_codec.c
> @@ -492,6 +492,9 @@ static int read_pin_defaults(struct hda_codec *codec)
>  		pin->nid = nid;
>  		pin->cfg = snd_hda_codec_read(codec, nid, 0,
>  					      AC_VERB_GET_CONFIG_DEFAULT, 0);
> +		/* all device entries are the same widget control so far
> +		 * fixme: if any codec is different, need fix here
> +		 */
>  		pin->ctrl = snd_hda_codec_read(codec, nid, 0,
>  					       AC_VERB_GET_PIN_WIDGET_CONTROL,
>  					       0);
> diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
> index 5987a31..67e191c 100644
> --- a/sound/pci/hda/patch_hdmi.c
> +++ b/sound/pci/hda/patch_hdmi.c
> @@ -144,7 +144,20 @@ struct hdmi_spec {
>  	struct snd_array cvts; /* struct hdmi_spec_per_cvt */
>  	hda_nid_t cvt_nids[4]; /* only for haswell fix */
>  
> +	/* num_pins is the number of virtual pins
> +	 * for example, there are 3 pins, and each pin
> +	 * has 4 device entries, then the num_pins is 12
> +	 */
>  	int num_pins;
> +	/* num_nids is the number of real pins
> +	 * In the above example, num_nids is 3
> +	 */
> +	int num_nids;
> +	/* dev_num is the number of device entries
> +	 * on each pin.
> +	 * In the above example, dev_num is 4
> +	 */
> +	int dev_num;
>  	struct snd_array pins; /* struct hdmi_spec_per_pin */
>  	struct hdmi_pcm pcm_rec[16];
>  	struct mutex pcm_lock;
> @@ -1248,6 +1261,10 @@ static void hdmi_setup_audio_infoframe(struct hda_codec *codec,
>  
>  static bool hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, int repoll);
>  
> +/* todo:
> + * To fully support DP MST, check_presence_and_report need param dev_id
> + * to tell which device entry occurs monitor connection event
> + */
>  static void check_presence_and_report(struct hda_codec *codec, hda_nid_t nid)
>  {
>  	struct hdmi_spec *spec = codec->spec;
> @@ -1271,6 +1288,11 @@ static void hdmi_intrinsic_event(struct hda_codec *codec, unsigned int res)
>  	struct hda_jack_tbl *jack;
>  	int dev_entry = (res & AC_UNSOL_RES_DE) >> AC_UNSOL_RES_DE_SHIFT;
>  
> +	/* assume DP MST uses dyn_pcm_assign and acomp and
> +	 * never comes here
> +	 * if DP MST supports unsol event, below code need
> +	 * consider dev_entry
> +	 */
>  	jack = snd_hda_jack_tbl_get_from_tag(codec, tag);
>  	if (!jack)
>  		return;
> @@ -1499,28 +1521,42 @@ static int intel_cvt_id_to_mux_idx(struct hdmi_spec *spec,
>   * by any other pins.
>   */
>  static void intel_not_share_assigned_cvt(struct hda_codec *codec,
> -			hda_nid_t pin_nid, int mux_idx)
> +					 hda_nid_t pin_nid,
> +					 int dev_id, int mux_idx)
>  {
>  	struct hdmi_spec *spec = codec->spec;
>  	hda_nid_t nid;
>  	int cvt_idx, curr;
>  	struct hdmi_spec_per_cvt *per_cvt;
> +	struct hdmi_spec_per_pin *per_pin;
> +	int pin_idx;
>  
>  	/* configure all pins, including "no physical connection" ones */
> -	for_each_hda_codec_node(nid, codec) {
> -		unsigned int wid_caps = get_wcaps(codec, nid);
> -		unsigned int wid_type = get_wcaps_type(wid_caps);
> +	for (pin_idx = 0; pin_idx <= spec->num_pins; pin_idx++) {

Is the condition correct?  i.e. pin_idx = spec->num_pins is valid?


> +		int dev_id_saved;
>  
> -		if (wid_type != AC_WID_PIN)
> +		per_pin = get_pin(spec, pin_idx);
> +		/* pin not connected to monitor
> +		 * no need to operate on it
> +		 */
> +		if (!per_pin->pcm)
>  			continue;
>  
> -		if (nid == pin_nid)
> +		if ((per_pin->pin_nid == pin_nid) &&
> +			(per_pin->dev_id == dev_id))
>  			continue;
>  
> +		nid = per_pin->pin_nid;
> +
> +		/* save the dev id for echo pin, and restore it when return */
> +		dev_id_saved = snd_hda_get_dev_select(codec, nid);
> +		snd_hda_set_dev_select(codec, nid, per_pin->dev_id);
>  		curr = snd_hda_codec_read(codec, nid, 0,
>  					  AC_VERB_GET_CONNECT_SEL, 0);
> -		if (curr != mux_idx)
> +		if (curr != mux_idx) {
> +			snd_hda_set_dev_select(codec, nid, dev_id_saved);
>  			continue;
> +		}
>  
>  		/* choose an unassigned converter. The conveters in the
>  		 * connection list are in the same order as in the codec.
> @@ -1537,12 +1573,13 @@ static void intel_not_share_assigned_cvt(struct hda_codec *codec,
>  				break;
>  			}
>  		}
> +		snd_hda_set_dev_select(codec, nid, dev_id_saved);
>  	}
>  }
>  
>  /* A wrapper of intel_not_share_asigned_cvt() */
>  static void intel_not_share_assigned_cvt_nid(struct hda_codec *codec,
> -			hda_nid_t pin_nid, hda_nid_t cvt_nid)
> +			hda_nid_t pin_nid, int dev_id, hda_nid_t cvt_nid)
>  {
>  	int mux_idx;
>  	struct hdmi_spec *spec = codec->spec;
> @@ -1557,7 +1594,7 @@ static void intel_not_share_assigned_cvt_nid(struct hda_codec *codec,
>  	 */
>  	mux_idx = intel_cvt_id_to_mux_idx(spec, cvt_nid);
>  	if (mux_idx >= 0)
> -		intel_not_share_assigned_cvt(codec, pin_nid, mux_idx);
> +		intel_not_share_assigned_cvt(codec, pin_nid, dev_id, mux_idx);
>  }
>  
>  /* called in hdmi_pcm_open when no pin is assigned to the PCM
> @@ -1585,7 +1622,7 @@ static int hdmi_pcm_open_no_pin(struct hda_pcm_stream *hinfo,
>  	per_cvt->assigned = 1;
>  	hinfo->nid = per_cvt->cvt_nid;
>  
> -	intel_not_share_assigned_cvt_nid(codec, 0, per_cvt->cvt_nid);
> +	intel_not_share_assigned_cvt_nid(codec, 0, 0, per_cvt->cvt_nid);
>  
>  	set_bit(pcm_idx, &spec->pcm_in_use);
>  	/* todo: setup spdif ctls assign */
> @@ -1661,13 +1698,15 @@ static int hdmi_pcm_open(struct hda_pcm_stream *hinfo,
>  	per_pin->cvt_nid = per_cvt->cvt_nid;
>  	hinfo->nid = per_cvt->cvt_nid;
>  
> +	snd_hda_set_dev_select(codec, per_pin->pin_nid, per_pin->dev_id);
>  	snd_hda_codec_write_cache(codec, per_pin->pin_nid, 0,
>  			    AC_VERB_SET_CONNECT_SEL,
>  			    mux_idx);
>  
>  	/* configure unused pins to choose other converters */
>  	if (is_haswell_plus(codec) || is_valleyview_plus(codec))
> -		intel_not_share_assigned_cvt(codec, per_pin->pin_nid, mux_idx);
> +		intel_not_share_assigned_cvt(codec, per_pin->pin_nid,
> +					     per_pin->dev_id, mux_idx);
>  
>  	snd_hda_spdif_ctls_assign(codec, pcm_idx, per_cvt->cvt_nid);
>  
> @@ -1737,13 +1776,13 @@ static int hdmi_find_pcm_slot(struct hdmi_spec *spec,
>  		return per_pin->pin_nid_idx;
>  
>  	/* have a second try; check the "reserved area" over num_pins */
> -	for (i = spec->num_pins; i < spec->pcm_used; i++) {
> +	for (i = spec->num_nids; i < spec->pcm_used; i++) {
>  		if (!test_bit(i, &spec->pcm_bitmap))
>  			return i;
>  	}
>  
>  	/* the last try; check the empty slots in pins */
> -	for (i = 0; i < spec->num_pins; i++) {
> +	for (i = 0; i < spec->num_nids; i++) {
>  		if (!test_bit(i, &spec->pcm_bitmap))
>  			return i;
>  	}
> @@ -1818,10 +1857,13 @@ static void hdmi_pcm_setup_pin(struct hdmi_spec *spec,
>  	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)
> +	if (mux_idx < per_pin->num_mux_nids) {
> +		snd_hda_set_dev_select(codec, per_pin->pin_nid,
> +				   per_pin->dev_id);
>  		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);
> @@ -1902,7 +1944,7 @@ static void update_eld(struct hda_codec *codec,
>  		if (is_haswell_plus(codec) || is_valleyview_plus(codec)) {
>  			intel_verify_pin_cvt_connect(codec, per_pin);
>  			intel_not_share_assigned_cvt(codec, per_pin->pin_nid,
> -						     per_pin->mux_idx);
> +				per_pin->dev_id, per_pin->mux_idx);
>  		}
>  
>  		hdmi_setup_audio_infoframe(codec, per_pin, per_pin->non_pcm);
> @@ -1996,6 +2038,7 @@ static struct snd_jack *pin_idx_to_jack(struct hda_codec *codec,
>  	if (per_pin->pcm_idx >= 0 && spec->dyn_pcm_assign)
>  		jack = spec->pcm_rec[per_pin->pcm_idx].jack;
>  	else if (!spec->dyn_pcm_assign) {
> +		//jack tbl not support mst
>  		jack_tbl = snd_hda_jack_tbl_get(codec, per_pin->pin_nid);
>  		if (jack_tbl)
>  			jack = jack_tbl->jack;
> @@ -2013,6 +2056,9 @@ static void sync_eld_via_acomp(struct hda_codec *codec,
>  	int size;
>  
>  	mutex_lock(&per_pin->lock);
> +	/* todo:
> +	 * to fully support DP MST, need add param dev_id
> +	 */
>  	size = snd_hdac_acomp_get_eld(&codec->bus->core, per_pin->pin_nid,
>  				      per_pin->dev_id, &eld->monitor_present,
>  				      eld->eld_buffer, ELD_MAX_SIZE);
> @@ -2079,7 +2125,7 @@ static void hdmi_repoll_eld(struct work_struct *work)
>  }
>  
>  static void intel_haswell_fixup_connect_list(struct hda_codec *codec,
> -					     hda_nid_t nid);
> +					     hda_nid_t nid, int dev_id);
>  
>  static int hdmi_add_pin(struct hda_codec *codec, hda_nid_t pin_nid)
>  {
> @@ -2088,38 +2134,59 @@ static int hdmi_add_pin(struct hda_codec *codec, hda_nid_t pin_nid)
>  	int pin_idx;
>  	struct hdmi_spec_per_pin *per_pin;
>  	int err;
> +	int dev_num, i;
>  
>  	caps = snd_hda_query_pin_caps(codec, pin_nid);
>  	if (!(caps & (AC_PINCAP_HDMI | AC_PINCAP_DP)))
>  		return 0;
>  
> +	/* For DP MST audio, Configuration Default is the same for
> +	 * all device entries on the same pin
> +	 */
>  	config = snd_hda_codec_get_pincfg(codec, pin_nid);
>  	if (get_defcfg_connect(config) == AC_JACK_PORT_NONE)
>  		return 0;
>  
> -	if (is_haswell_plus(codec))
> -		intel_haswell_fixup_connect_list(codec, pin_nid);
> -
> -	pin_idx = spec->num_pins;
> -	per_pin = snd_array_new(&spec->pins);
> -	if (!per_pin)
> -		return -ENOMEM;
> -
> -	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 = get_hdmi_pcm(spec, pin_idx);
> -		per_pin->pcm_idx = pin_idx;
> +	if (is_haswell_plus(codec)) {
> +		dev_num = 3;
> +		spec->dev_num = 3;
> +	} else {
> +		dev_num = snd_hda_get_num_devices(codec, pin_nid) + 1;

Does snd_hda_get_num_devices() return 0?  What if it's no mst-capable
codec, and what if a MST-capable codec having a single device?


> +		/* spec->dev_num is the maxinum number of device entries
> +		 * among all the pins
> +		 */
> +		spec->dev_num = (spec->dev_num > dev_num) ?
> +			spec->dev_num : dev_num;
>  	}

The presence of is_haswell_*() or such macro in every place makes
really hard to maintain the code.  In my new patchset (I'll post soon
later), I tried to reduce these usages, and split the functions to
Intel-specific ones.  Let's try not to contaminate too much again.


Takashi
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel



[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux