On Fri, 13 Nov 2015 09:56:30 +0100,
libin.yang@xxxxxxxxxxxxxxx wrote:
>
> From: Libin Yang <libin.yang@xxxxxxxxxxxxxxx>
>
> Begin to support dynamic PCM assignment to pin in
> hdmi audio driver.
>
> This means PCM will not be statically bound with pin.
> When there is a monitor connected, the corresponding pin
> will try to find a proper PCM to bind. When the monitor
> is disconnected, the corresponding pin will unbind
> the PCM. This helps to reduce the PCM number when there
> are many pins (device entries in DP MST mode) and only
> a few of them work at the same time.
>
> This patch adds the pcm member in struct hdmi_spec_per_pin.
> When PCM is dynamically bound to the pin, the member pcm
> will pointer to the corresponding pcm_rec[] in hdmi_spec,
> which means the hda_pcm is bound to the pin.
>
> Signed-off-by: Libin Yang <libin.yang@xxxxxxxxxxxxxxx>
> ---
> sound/pci/hda/patch_hdmi.c | 26 +++++++++++++++++++++-----
> 1 file changed, 21 insertions(+), 5 deletions(-)
>
> diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
> index f503a88..f3363b8 100644
> --- a/sound/pci/hda/patch_hdmi.c
> +++ b/sound/pci/hda/patch_hdmi.c
> @@ -82,6 +82,7 @@ struct hdmi_spec_per_pin {
> struct mutex lock;
> struct delayed_work work;
> struct snd_kcontrol *eld_ctl;
> + struct hda_pcm *pcm; /* pointer to spec->pcm_rec[n] dynamically*/
> int repoll_count;
> bool setup; /* the stream has been set up by prepare callback */
> int channels; /* current number of channels */
> @@ -140,7 +141,8 @@ struct hdmi_spec {
> struct hdmi_ops ops;
>
> bool dyn_pin_out;
> -
> + bool dyn_pcm_assign;
> + struct mutex pcm_lock;
> /*
> * Non-generic VIA/NVIDIA specific
> */
> @@ -378,13 +380,26 @@ static int hinfo_to_pin_index(struct hda_codec *codec,
> struct hda_pcm_stream *hinfo)
> {
> struct hdmi_spec *spec = codec->spec;
> + struct hdmi_spec_per_pin *per_pin;
> int pin_idx;
>
> - for (pin_idx = 0; pin_idx < spec->num_pins; pin_idx++)
> - if (get_pcm_rec(spec, pin_idx)->stream == hinfo)
> - return pin_idx;
> + if (!spec->dyn_pcm_assign) {
> + for (pin_idx = 0; pin_idx < spec->num_pins; pin_idx++)
> + if (get_pcm_rec(spec, pin_idx)->stream == hinfo)
> + return pin_idx;
> + } else {
> + mutex_lock(&spec->pcm_lock);
> + for (pin_idx = 0; pin_idx < spec->num_pins; pin_idx++) {
> + per_pin = get_pin(spec, pin_idx);
> + if (per_pin->pcm && per_pin->pcm->stream == hinfo) {
> + mutex_unlock(&spec->pcm_lock);
> + return pin_idx;
> + }
> + }
> + mutex_unlock(&spec->pcm_lock);
You can assign per_pin->pcm statically at init time when
dyn_pcm_assign=false. Then the same code is used in the above.
The need of spec->pcm_lock isn't clear at this moment. I guess you're
trying to avoid the race of dynamic pin assignment and release. But
then the code above doesn't protect anything because you return the
pin_idx as an available value after the unlock. Then the pin release
can happen after this point while you're accessing with the pin_idx.
So, better to consider the implementation of lock or other mechanism
once when the whole picture is ready.
Takashi
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
