On 2019/5/6 下午5:31, Takashi Iwai wrote:
On Mon, 06 May 2019 11:25:16 +0200,
Yang, Libin wrote:
Hi Jaroslav,
-----Original Message-----
From: Jaroslav Kysela [mailto:perex@xxxxxxxx]
Sent: Monday, May 6, 2019 5:04 PM
To: Yang, Libin <libin.yang@xxxxxxxxx>; alsa-devel@xxxxxxxxxxxxxxxx
Cc: tiwai@xxxxxxx; pierre-louis.bossart@xxxxxxxxxxxxxxx; broonie@xxxxxxxxxx;
Hui Wang <hui.wang@xxxxxxxxxxxxx>; Lin, Mengdong
<mengdong.lin@xxxxxxxxx>; Wang, Rander <rander.wang@xxxxxxxxx>
Subject: Re: [RFC PATCH] ASoC: codec: hdac_hdmi: no checking
monitor in hw_params
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.
We may still need Jack control for this case. This is my second plan to enable
Jack control. But even we enable Jack control, it seems it is still easy to use UCM.
This is because ASoC control names don't follow legacy HDA rule. This means
we may need to change other controls name besides HDMI in the driver or we
may need to add more configures in pulseaudio/alsa-mixer/paths.
Ah, I overlooked that hdac_hdmi chose the own jack control names.
That's basically useless, yeah.
Hi Mengdong & Hui,
Do you have more comments for Jack control or UCM?
I guess a UCM profile would handle the jack detection as well, by
specifying the right kcontrol names, right?
Yes, probably it can work. set a HD-Audio Jack name in the
SectionDevice, and monitor the ASoC jack name via JackControl. like below:
SectionDevice."Jack HDMI/DP,pcm=3" {
Comment "HDMI/DP Audio #0"
Value {
CaptureChannels "2"
CapturePCM "hw:${card_name},#{subdev}"
JackControl "${ASoC_HDMI_Jack_Name} Jack"
}
...
Thanks,
Hui.
thanks,
Takashi
Regards,
Libin
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
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
