[alsa-devel] [RFC PATCH] ASoC: codec: hdac_hdmi: no checking monitor in hw_params
Hui Wang
hui.wang at canonical.com
Mon May 6 12:58:46 CEST 2019
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 at perex.cz]
>>> Sent: Monday, May 6, 2019 5:04 PM
>>> To: Yang, Libin <libin.yang at intel.com>; alsa-devel at alsa-project.org
>>> Cc: tiwai at suse.de; pierre-louis.bossart at linux.intel.com; broonie at kernel.org;
>>> Hui Wang <hui.wang at canonical.com>; Lin, Mengdong
>>> <mengdong.lin at intel.com>; Wang, Rander <rander.wang at intel.com>
>>> Subject: Re: [alsa-devel] [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 at perex.cz]
>>>>> Sent: Monday, May 6, 2019 4:20 PM
>>>>> To: Yang, Libin <libin.yang at intel.com>; alsa-devel at alsa-project.org
>>>>> Cc: tiwai at suse.de; pierre-louis.bossart at linux.intel.com;
>>>>> broonie at kernel.org; subhransu.s.prusty at intel.com;
>>>>> samreen.nilofer at intel.com
>>>>> Subject: Re: [alsa-devel] [RFC PATCH] ASoC: codec: hdac_hdmi: no
>>>>> checking monitor in hw_params
>>>>>
>>>>> 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.
>>>> 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 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.
>>>
>>> --
>>> Jaroslav Kysela <perex at perex.cz>
>>> Linux Sound Maintainer; ALSA Project; Red Hat, Inc.
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel at alsa-project.org
> https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
>
More information about the Alsa-devel
mailing list