On Wed, 11 Nov 2015 10:02:14 +0100,
Takashi Iwai wrote:
>
> On Wed, 11 Nov 2015 09:39:09 +0100,
> libin.yang@xxxxxxxxxxxxxxx wrote:
> >
> > From: Libin Yang <libin.yang@xxxxxxxxxxxxxxx>
> >
> > Add a flag that user can decide to stop HDMI/DP playback when
> > the corresponding monitor is disconnected and refuse to open PCM
> > if there is no monitor connected.
> >
> > Background:
> > When a monitor is disconnected and a new monitor is connected,
> > the parameters of the 2 monitors may be different. Audio driver
> > need handle this situation.
> >
> > Besides, stopping playback when monitor is disconnected will
> > help to save the power.
> >
> > Signed-off-by: Libin Yang <libin.yang@xxxxxxxxxxxxxxx>
>
> Thanks. Below are just nitpicking, so let's test this patch at first,
> especially to see whether it has any significant influence on PA, then
> respin with the fixes.
>
> David, care to check in your side, too?
So I tested this with PA 7.1, and it failed, unfortunately.
In short:
- PA needs the PCM access at probe. If it gets an error, the device
will be never enumerated again
- PA removes the device when it's disconnected. The PCM stop with
DISCONNECT state leads to the device disappearance.
Takashi
>
> > ---
> > sound/pci/hda/patch_hdmi.c | 34 ++++++++++++++++++++++++++++++++++
> > 1 file changed, 34 insertions(+)
> >
> > diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
> > index f503a88..4f5023a 100644
> > --- a/sound/pci/hda/patch_hdmi.c
> > +++ b/sound/pci/hda/patch_hdmi.c
> > @@ -47,6 +47,11 @@ static bool static_hdmi_pcm;
> > module_param(static_hdmi_pcm, bool, 0644);
> > MODULE_PARM_DESC(static_hdmi_pcm, "Don't restrict PCM parameters per ELD info");
> >
> > +static bool hdmi_pcm_stop_on_disconnect;
> > +module_param(hdmi_pcm_stop_on_disconnect, bool, 0644);
> > +MODULE_PARM_DESC(hdmi_pcm_stop_on_disconnect,
> > + "Stop PCM when monitor is disconnected");
>
> This codec driver may be used for multiple devices, e.g. an onboard
> GPU and a discrete GPU. Whether a single flag is best is a slight
> concern. Though, as a test patch, it certainly suffices.
>
> > #define is_haswell(codec) ((codec)->core.vendor_id == 0x80862807)
> > #define is_broadwell(codec) ((codec)->core.vendor_id == 0x80862808)
> > #define is_skylake(codec) ((codec)->core.vendor_id == 0x80862809)
> > @@ -72,6 +77,7 @@ struct hdmi_spec_per_cvt {
> >
> > struct hdmi_spec_per_pin {
> > hda_nid_t pin_nid;
> > + int pin_idx;
> > int num_mux_nids;
> > hda_nid_t mux_nids[HDA_MAX_CONNECTIONS];
> > int mux_idx;
> > @@ -1456,6 +1462,9 @@ static int hdmi_pcm_open(struct hda_pcm_stream *hinfo,
> > per_pin = get_pin(spec, pin_idx);
> > eld = &per_pin->sink_eld;
> >
> > + if (hdmi_pcm_stop_on_disconnect && !eld->eld_valid)
> > + return -ENODEV;
>
> An error code might influence on behavior, so let's check it carefully
> while testing.
>
>
> > err = hdmi_choose_cvt(codec, pin_idx, &cvt_idx, &mux_idx);
> > if (err < 0)
> > return err;
> > @@ -1529,6 +1538,28 @@ static int hdmi_read_pin_conn(struct hda_codec *codec, int pin_idx)
> > return 0;
> > }
> >
> > +static void hdmi_pcm_stop(struct hdmi_spec *spec,
> > + struct hdmi_spec_per_pin *per_pin)
> > +{
> > + struct hda_codec *codec = per_pin->codec;
> > + struct snd_pcm_substream *substream;
> > + int pin_idx = per_pin->pin_idx;
> > +
> > + if (!hdmi_pcm_stop_on_disconnect)
> > + return;
> > +
> > + substream = get_pcm_rec(spec, pin_idx)->pcm->streams[0].substream;
> > +
> > + snd_pcm_stream_lock_irq(substream);
> > + if (substream && substream->runtime &&
> > + snd_pcm_running(substream)) {
>
> substream NULL check has to be before the lock.
>
>
> > + codec_info(codec,
> > + "HDMI: monitor disconnected, try to stop playback\n");
>
> The message is good for testing, but no need to spam at each
> disconnect in the production state. Use codec_dbg() in the final
> patch.
>
>
> thanks,
>
> Takashi
>
> > + snd_pcm_stop(substream, SNDRV_PCM_STATE_DISCONNECTED);
> > + }
> > + snd_pcm_stream_unlock_irq(substream);
> > +}
> > +
> > static bool hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, int repoll)
> > {
> > struct hda_jack_tbl *jack;
> > @@ -1586,6 +1617,8 @@ static bool hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, int repoll)
> > }
> > }
> >
> > + if (!eld->eld_valid)
> > + hdmi_pcm_stop(spec, per_pin);
> > if (pin_eld->eld_valid != eld->eld_valid)
> > eld_changed = true;
> >
> > @@ -1680,6 +1713,7 @@ static int hdmi_add_pin(struct hda_codec *codec, hda_nid_t pin_nid)
> > return -ENOMEM;
> >
> > per_pin->pin_nid = pin_nid;
> > + per_pin->pin_idx = pin_idx;
> > per_pin->non_pcm = false;
> >
> > err = hdmi_read_pin_conn(codec, pin_idx);
> > --
> > 1.9.1
> >
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
