Dne 06. 05. 19 v 10:46 Yang, Libin napsal(a): > Add Mengdong, Hui, Rander > > Hi Jaroslav, > >> -----Original Message----- >> From: Jaroslav Kysela [mailto:perex@xxxxxxxx] >> Sent: Monday, May 6, 2019 4:20 PM >> To: Yang, Libin <libin.yang@xxxxxxxxx>; alsa-devel@xxxxxxxxxxxxxxxx >> Cc: tiwai@xxxxxxx; pierre-louis.bossart@xxxxxxxxxxxxxxx; broonie@xxxxxxxxxx; >> subhransu.s.prusty@xxxxxxxxx; samreen.nilofer@xxxxxxxxx >> Subject: Re: [RFC PATCH] ASoC: codec: hdac_hdmi: no checking >> monitor in hw_params >> >> Dne 06. 05. 19 v 8:59 libin.yang@xxxxxxxxx napsal(a): >>> From: Libin Yang <libin.yang@xxxxxxxxx> >>> >>> This patch move the check of monitor from hw_params to trigger callback. >>> >>> The original code will check the monitor presence in hw_params. If the >>> monitor doesn't exist, hw_params will return -ENODEV. Mostly this is OK. >>> >>> However, pulseaudio will check the pcm devices when kernel is booting up. >>> It will try to open, set hw_params, prepare such pcm devices. We can't >>> guarantee that the monitor will be connected when kernel is booting up. >>> Especially, hdac_hdmi will export 3 pcms at most. It's hard to say >>> users will connect 3 monitors to the HDMI/DP ports. This will cause >>> pulseaudio fail in parsing the pcm devices because the driver will >>> return -ENODEV in hw_params. >>> >>> This patch tries to move the check of monitor presence into trigger >>> callback. This can "trick" the pulseaudio the pcm is ready. >>> >>> This bug is found when we try to enable HDMI detection in >>> gnome-sound-setting for ASoC hdac_hdmi. After we enable the hdmi in >>> UCM, pulseaudio will try to parse the hdmi pcm devices. It will cause >>> failure if there are no monitors connected. >> >> I don't like this solution much. PA should use the Jack control to add the >> devices dynamically and avoid probing when the Jack control is false. > > Before we decided to use UCM, we did some investigation on Jack controls. > And we found we need do much more changes in driver to support Jack > Controls. How do you handle the dynamic monitor configuration (like when user disconnects the monitor on-the-fly) then? The control interface can notify the state change through the Jack controls. The PCM interface does not handle this. Jaroslav > > Regards, > Libin > >> >> Jaroslav >> >>> >>> Signed-off-by: Libin Yang <libin.yang@xxxxxxxxx> >>> --- >>> sound/soc/codecs/hdac_hdmi.c | 44 >>> +++++++++++++++++++++++++++++++------------- >>> 1 file changed, 31 insertions(+), 13 deletions(-) >>> >>> diff --git a/sound/soc/codecs/hdac_hdmi.c >>> b/sound/soc/codecs/hdac_hdmi.c index 4de1fbf..f482e09 100644 >>> --- a/sound/soc/codecs/hdac_hdmi.c >>> +++ b/sound/soc/codecs/hdac_hdmi.c >>> @@ -455,24 +455,11 @@ static int hdac_hdmi_set_hw_params(struct >> snd_pcm_substream *substream, >>> struct snd_pcm_hw_params *hparams, struct snd_soc_dai *dai) { >>> struct hdac_hdmi_priv *hdmi = snd_soc_dai_get_drvdata(dai); >>> - struct hdac_device *hdev = hdmi->hdev; >>> struct hdac_hdmi_dai_port_map *dai_map; >>> - struct hdac_hdmi_port *port; >>> struct hdac_hdmi_pcm *pcm; >>> int format; >>> >>> dai_map = &hdmi->dai_map[dai->id]; >>> - port = dai_map->port; >>> - >>> - if (!port) >>> - return -ENODEV; >>> - >>> - if ((!port->eld.monitor_present) || (!port->eld.eld_valid)) { >>> - dev_err(&hdev->dev, >>> - "device is not configured for this pin:port%d:%d\n", >>> - port->pin->nid, port->id); >>> - return -ENODEV; >>> - } >>> >>> format = snd_hdac_calc_stream_format(params_rate(hparams), >>> params_channels(hparams), >> params_format(hparams), @@ -630,6 >>> +617,36 @@ static void hdac_hdmi_pcm_close(struct snd_pcm_substream >> *substream, >>> dai_map->port = NULL; >>> } >>> >>> +static int hdac_hdmi_pcm_trigger(struct snd_pcm_substream *substream, >> int cmd, >>> + struct snd_soc_dai *dai) >>> +{ >>> + struct hdac_hdmi_port *port; >>> + struct hdac_hdmi_dai_port_map *dai_map; >>> + struct hdac_hdmi_priv *hdmi = snd_soc_dai_get_drvdata(dai); >>> + struct hdac_device *hdev = hdmi->hdev; >>> + >>> + /* >>> + * When start, if there is no monitor, >>> + * It should not start audio. >>> + */ >>> + if (cmd == SNDRV_PCM_TRIGGER_START) { >>> + dai_map = &hdmi->dai_map[dai->id]; >>> + port = dai_map->port; >>> + >>> + if (!port) >>> + return -ENODEV; >>> + >>> + if ((!port->eld.monitor_present) || (!port->eld.eld_valid)) { >>> + dev_err(&hdev->dev, >>> + "device is not configured for this >> pin:port%d:%d\n", >>> + port->pin->nid, port->id); >>> + return -ENODEV; >>> + } >>> + } >>> + >>> + return 0; >>> +} >>> + >>> static int >>> hdac_hdmi_query_cvt_params(struct hdac_device *hdev, struct >>> hdac_hdmi_cvt *cvt) { @@ -1389,6 +1406,7 @@ static const struct >>> snd_soc_dai_ops hdmi_dai_ops = { >>> .startup = hdac_hdmi_pcm_open, >>> .shutdown = hdac_hdmi_pcm_close, >>> .hw_params = hdac_hdmi_set_hw_params, >>> + .trigger = hdac_hdmi_pcm_trigger, >>> .set_tdm_slot = hdac_hdmi_set_tdm_slot, }; >>> >>> >> >> >> -- >> Jaroslav Kysela <perex@xxxxxxxx> >> Linux Sound Maintainer; ALSA Project; Red Hat, Inc. -- Jaroslav Kysela <perex@xxxxxxxx> Linux Sound Maintainer; ALSA Project; Red Hat, Inc. _______________________________________________ Alsa-devel mailing list Alsa-devel@xxxxxxxxxxxxxxxx https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
