> -----Original Message----- > From: Takashi Iwai [mailto:tiwai@xxxxxxx] > Sent: Wednesday, November 11, 2015 10:13 PM > To: libin.yang@xxxxxxxxxxxxxxx > Cc: David Henningsson; alsa-devel@xxxxxxxxxxxxxxxx; > mengdong.lin@xxxxxxxxxxxxxxx; Yang, Libin > Subject: Re: [PATCH] ALSA - hda: hdmi flag to stop playback when > monitor is disconnected > > 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 Thanks for test. If so, should we re-write the hdmi_pcm_open() and generic_hdmi_playback_pcm_prepare() function to make the probe works? Or not support dynamic pcm assignment? > - PA removes the device when it's disconnected. The PCM stop with > DISCONNECT state leads to the device disappearance. If we can't use DISCONNECT, what else can we use? Or we can't stop PCM when monitor is disconnected? > > > 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
