[alsa-devel] [RFC PATCH 1/2] ALSA: hda - hdmi playback without monitor in dynamic pcm bind mode
Takashi Iwai
tiwai at suse.de
Wed Nov 18 12:13:12 CET 2015
On Wed, 18 Nov 2015 09:46:58 +0100,
libin.yang at linux.intel.com wrote:
>
> From: Libin Yang <libin.yang at linux.intel.com>
>
> Pulseaudio requires open pcm successfully when probing.
>
> This patch handles playback without monitor in dynamic pcm assignment
> mode. It tries to open/prepare/close pcm successfully even there is
> no pin bound to the PCM. On the meantime, it will try to find a proper
> converter for the PCM.
>
> Signed-off-by: Libin Yang <libin.yang at linux.intel.com>
> ---
> sound/pci/hda/patch_hdmi.c | 163 ++++++++++++++++++++++++++++++++++++++++++---
> 1 file changed, 152 insertions(+), 11 deletions(-)
>
> diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
> index 92e05fb..12fc506 100644
> --- a/sound/pci/hda/patch_hdmi.c
> +++ b/sound/pci/hda/patch_hdmi.c
> @@ -135,6 +135,7 @@ struct hdmi_spec {
> int num_pins;
> struct snd_array pins; /* struct hdmi_spec_per_pin */
> struct hda_pcm *pcm_rec[16];
> + struct mutex pcm_lock;
Ideally this should be a rw semaphore so that it won't block too
much. But we can optimize it later.
> unsigned int channels_max; /* max over all cvts */
>
> struct hdmi_eld temp_eld;
> @@ -1388,6 +1389,17 @@ static void intel_verify_pin_cvt_connect(struct hda_codec *codec,
> mux_idx);
> }
>
> +static int intel_cvt_id_to_mux_idx(struct hdmi_spec *spec,
> + hda_nid_t cvt_nid)
> +{
> + int i;
> +
> + for (i = 0; i < spec->num_cvts; i++)
> + if (spec->cvt_nids[i] == cvt_nid)
> + return i;
> + return -EINVAL;
> +}
This is always a couple with intel_not_share_assigned_cvt().
So better to have a helper doing both in a shot.
(And give a comment what's doing.)
> +
> /* Intel HDMI workaround to fix audio routing issue:
> * For some Intel display codecs, pins share the same connection list.
> * So a conveter can be selected by multiple pins and playback on any of these
> @@ -1439,6 +1451,77 @@ static void intel_not_share_assigned_cvt(struct hda_codec *codec,
> }
> }
>
> +static int hdmi_find_available_cvt(struct hda_codec *codec,
> + int *cvt_id)
> +{
> + struct hdmi_spec *spec = codec->spec;
> + struct hdmi_spec_per_cvt *per_cvt = NULL;
> + int cvt_idx;
> +
> + /* Dynamically assign converter to stream */
> + for (cvt_idx = 0; cvt_idx < spec->num_cvts; cvt_idx++) {
> + per_cvt = get_cvt(spec, cvt_idx);
> +
> + /* Must not already be assigned */
> + if (!per_cvt->assigned)
> + break;
> + }
> +
> + /* No free converters */
> + if (cvt_idx == spec->num_cvts)
> + return -EBUSY;
> +
> + if (cvt_id)
> + *cvt_id = cvt_idx;
> +
> + return 0;
> +}
This could be merged into hdmi_choose_cvt(). Let per_pin = NULL if
pin_idx < 0, and skip the mux check, for example.
> +
> +/* called in hdmi_pcm_open when no pin is assigned to the PCM
> + * in dyn_pcm_assign mode.
> + */
> +static int hdmi_pcm_open_no_pin(struct hda_pcm_stream *hinfo,
> + struct hda_codec *codec,
> + struct snd_pcm_substream *substream)
> +{
> + struct hdmi_spec *spec = codec->spec;
> + struct snd_pcm_runtime *runtime = substream->runtime;
> + int cvt_idx, mux_idx;
> + struct hdmi_spec_per_cvt *per_cvt = NULL;
> + int err;
> +
> + err = hdmi_find_available_cvt(codec, &cvt_idx);
> + if (err)
> + return err;
> +
> + 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);
Don't forget the hsw+/vlv+ condition check like in other places.
Or maybe better put the check into the callee, the new helper
function.
Takashi
> +
> + /* todo: setup spdif ctls assign */
> +
> + /* Initially set the converter's capabilities */
> + hinfo->channels_min = per_cvt->channels_min;
> + hinfo->channels_max = per_cvt->channels_max;
> + hinfo->rates = per_cvt->rates;
> + hinfo->formats = per_cvt->formats;
> + hinfo->maxbps = per_cvt->maxbps;
> +
> + /* Store the updated parameters */
> + runtime->hw.channels_min = hinfo->channels_min;
> + runtime->hw.channels_max = hinfo->channels_max;
> + runtime->hw.formats = hinfo->formats;
> + runtime->hw.rates = hinfo->rates;
> +
> + snd_pcm_hw_constraint_step(substream->runtime, 0,
> + SNDRV_PCM_HW_PARAM_CHANNELS, 2);
> + return 0;
> +}
> +
> /*
> * HDA PCM callbacks
> */
> @@ -1455,19 +1538,36 @@ static int hdmi_pcm_open(struct hda_pcm_stream *hinfo,
> int err;
>
> /* Validate hinfo */
> + mutex_lock(&spec->pcm_lock);
> pin_idx = hinfo_to_pin_index(codec, hinfo);
> - if (snd_BUG_ON(pin_idx < 0))
> - return -EINVAL;
> - per_pin = get_pin(spec, pin_idx);
> - eld = &per_pin->sink_eld;
> + if (!spec->dyn_pcm_assign) {
> + if (snd_BUG_ON(pin_idx < 0)) {
> + mutex_unlock(&spec->pcm_lock);
> + return -EINVAL;
> + }
> + } else {
> + /* no pin is assigned to the PCM
> + * PA need pcm open successfully when probe
> + */
> + if (pin_idx < 0) {
> + err = hdmi_pcm_open_no_pin(hinfo, codec, substream);
> + mutex_unlock(&spec->pcm_lock);
> + return err;
> + }
> + }
>
> err = hdmi_choose_cvt(codec, pin_idx, &cvt_idx, &mux_idx);
> - if (err < 0)
> + if (err < 0) {
> + mutex_unlock(&spec->pcm_lock);
> return err;
> + }
>
> per_cvt = get_cvt(spec, cvt_idx);
> /* Claim converter */
> per_cvt->assigned = 1;
> +
> +
> + per_pin = get_pin(spec, pin_idx);
> per_pin->cvt_nid = per_cvt->cvt_nid;
> hinfo->nid = per_cvt->cvt_nid;
>
> @@ -1488,6 +1588,7 @@ static int hdmi_pcm_open(struct hda_pcm_stream *hinfo,
> hinfo->formats = per_cvt->formats;
> hinfo->maxbps = per_cvt->maxbps;
>
> + eld = &per_pin->sink_eld;
> /* Restrict capabilities by ELD if this isn't disabled */
> if (!static_hdmi_pcm && eld->eld_valid) {
> snd_hdmi_eld_update_pcm_info(&eld->info, hinfo);
> @@ -1496,10 +1597,12 @@ static int hdmi_pcm_open(struct hda_pcm_stream *hinfo,
> per_cvt->assigned = 0;
> hinfo->nid = 0;
> snd_hda_spdif_ctls_unassign(codec, pin_idx);
> + mutex_unlock(&spec->pcm_lock);
> return -ENODEV;
> }
> }
>
> + mutex_unlock(&spec->pcm_lock);
> /* Store the updated parameters */
> runtime->hw.channels_min = hinfo->channels_min;
> runtime->hw.channels_max = hinfo->channels_max;
> @@ -1803,13 +1906,38 @@ static int generic_hdmi_playback_pcm_prepare(struct hda_pcm_stream *hinfo,
> {
> hda_nid_t cvt_nid = hinfo->nid;
> struct hdmi_spec *spec = codec->spec;
> - int pin_idx = hinfo_to_pin_index(codec, hinfo);
> - struct hdmi_spec_per_pin *per_pin = get_pin(spec, pin_idx);
> - hda_nid_t pin_nid = per_pin->pin_nid;
> + int pin_idx;
> + struct hdmi_spec_per_pin *per_pin;
> + hda_nid_t pin_nid;
> struct snd_pcm_runtime *runtime = substream->runtime;
> struct i915_audio_component *acomp = codec->bus->core.audio_component;
> bool non_pcm;
> + int mux_idx;
> int pinctl;
> + int err;
> +
> + mutex_lock(&spec->pcm_lock);
> + pin_idx = hinfo_to_pin_index(codec, hinfo);
> + if (spec->dyn_pcm_assign && pin_idx < 0) {
> + /* when dyn_pcm_assign and pcm is not bound to a pin
> + * skip pin setup and return 0 to make audio playback
> + * be ongoing
> + */
> + if (is_haswell_plus(codec) || is_valleyview_plus(codec)) {
> + mux_idx = intel_cvt_id_to_mux_idx(spec, cvt_nid);
> + if (mux_idx > 0)
> + intel_not_share_assigned_cvt(codec, 0, mux_idx);
> + }
> + snd_hda_codec_setup_stream(codec, cvt_nid,
> + stream_tag, 0, format);
> + mutex_unlock(&spec->pcm_lock);
> + return 0;
> + }
> +
> + if (snd_BUG_ON(pin_idx < 0))
> + return -EINVAL;
> + per_pin = get_pin(spec, pin_idx);
> + pin_nid = per_pin->pin_nid;
>
> if (is_haswell_plus(codec) || is_valleyview_plus(codec)) {
> /* Verify pin:cvt selections to avoid silent audio after S3.
> @@ -1838,7 +1966,7 @@ static int generic_hdmi_playback_pcm_prepare(struct hda_pcm_stream *hinfo,
>
> hdmi_setup_audio_infoframe(codec, per_pin, non_pcm);
> mutex_unlock(&per_pin->lock);
> -
> + mutex_unlock(&spec->pcm_lock);
> if (spec->dyn_pin_out) {
> pinctl = snd_hda_codec_read(codec, pin_nid, 0,
> AC_VERB_GET_PIN_WIDGET_CONTROL, 0);
> @@ -1847,7 +1975,10 @@ static int generic_hdmi_playback_pcm_prepare(struct hda_pcm_stream *hinfo,
> pinctl | PIN_OUT);
> }
>
> - return spec->ops.setup_stream(codec, cvt_nid, pin_nid, stream_tag, format);
> + err = spec->ops.setup_stream(codec, cvt_nid, pin_nid,
> + stream_tag, format);
> + mutex_unlock(&spec->pcm_lock);
> + return err;
> }
>
> static int generic_hdmi_playback_pcm_cleanup(struct hda_pcm_stream *hinfo,
> @@ -1878,9 +2009,17 @@ static int hdmi_pcm_close(struct hda_pcm_stream *hinfo,
> per_cvt->assigned = 0;
> hinfo->nid = 0;
>
> + mutex_lock(&spec->pcm_lock);
> pin_idx = hinfo_to_pin_index(codec, hinfo);
> - if (snd_BUG_ON(pin_idx < 0))
> + if (spec->dyn_pcm_assign && pin_idx < 0) {
> + mutex_unlock(&spec->pcm_lock);
> + return 0;
> + }
> +
> + if (snd_BUG_ON(pin_idx < 0)) {
> + mutex_unlock(&spec->pcm_lock);
> return -EINVAL;
> + }
> per_pin = get_pin(spec, pin_idx);
>
> if (spec->dyn_pin_out) {
> @@ -1900,6 +2039,7 @@ static int hdmi_pcm_close(struct hda_pcm_stream *hinfo,
> per_pin->setup = false;
> per_pin->channels = 0;
> mutex_unlock(&per_pin->lock);
> + mutex_unlock(&spec->pcm_lock);
> }
>
> return 0;
> @@ -2370,6 +2510,7 @@ static int patch_generic_hdmi(struct hda_codec *codec)
> return -ENOMEM;
>
> spec->ops = generic_standard_hdmi_ops;
> + mutex_init(&spec->pcm_lock);
> codec->spec = spec;
> hdmi_array_init(spec, 4);
>
> --
> 1.9.1
>
More information about the Alsa-devel
mailing list