All of lore.kernel.org
 help / color / mirror / Atom feed
From: libin.yang@intel.com
To: alsa-devel@alsa-project.org
Cc: Libin Yang <libin.yang@intel.com>,
	tiwai@suse.de, pierre-louis.bossart@linux.intel.com,
	broonie@kernel.org, subhransu.s.prusty@intel.com,
	samreen.nilofer@intel.com
Subject: [RFC PATCH] ASoC: codec: hdac_hdmi: no checking monitor in hw_params
Date: Mon,  6 May 2019 14:59:20 +0800	[thread overview]
Message-ID: <1557125960-29353-1-git-send-email-libin.yang@intel.com> (raw)

From: Libin Yang <libin.yang@intel.com>

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.

Signed-off-by: Libin Yang <libin.yang@intel.com>
---
 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,
 };
 
-- 
2.7.4

             reply	other threads:[~2019-05-06  7:18 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-06  6:59 libin.yang [this message]
2019-05-06  8:19 ` [RFC PATCH] ASoC: codec: hdac_hdmi: no checking monitor in hw_params Jaroslav Kysela
2019-05-06  8:39   ` Takashi Iwai
2019-05-06  8:56     ` Yang, Libin
2019-05-06  9:01       ` Takashi Iwai
2019-05-06  9:13         ` Yang, Libin
2019-05-06  8:46   ` Yang, Libin
2019-05-06  9:03     ` Jaroslav Kysela
2019-05-06  9:25       ` Yang, Libin
2019-05-06  9:31         ` Takashi Iwai
2019-05-06 10:58           ` Hui Wang
2019-05-06  9:25       ` Takashi Iwai
2019-05-06 15:37 ` Pierre-Louis Bossart
2019-05-06 15:41   ` Takashi Iwai
2019-05-06 15:47     ` Pierre-Louis Bossart
2019-05-06 15:50       ` Takashi Iwai
2019-05-06 15:54         ` Pierre-Louis Bossart
2019-05-07  1:29           ` Yang, Libin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1557125960-29353-1-git-send-email-libin.yang@intel.com \
    --to=libin.yang@intel.com \
    --cc=alsa-devel@alsa-project.org \
    --cc=broonie@kernel.org \
    --cc=pierre-louis.bossart@linux.intel.com \
    --cc=samreen.nilofer@intel.com \
    --cc=subhransu.s.prusty@intel.com \
    --cc=tiwai@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.