[alsa-devel] [RFC PATCH] ASoC: codec: hdac_hdmi: no checking monitor in hw_params

[Jaroslav Kysela]

Dne 06. 05. 19 v 8:59 libin.yang at intel.com napsal(a):
> From: Libin Yang <libin.yang at 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.

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.

						Jaroslav

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


-- 
Jaroslav Kysela <perex at perex.cz>
Linux Sound Maintainer; ALSA Project; Red Hat, Inc.