diff mbox series

ALSA: hda/hdmi: Add pins with jack detection support

Message ID 20200804072926.16897-1-kai.heng.feng@canonical.com (mailing list archive)
State New, archived
Headers show
Series ALSA: hda/hdmi: Add pins with jack detection support | expand

Commit Message

Kai-Heng Feng Aug. 4, 2020, 7:29 a.m. UTC
HDMI on some platforms doesn't enable audio support because its Port
Connectivity [31:30] is set to AC_JACK_PORT_NONE:
Node 0x05 [Pin Complex] wcaps 0x40778d: 8-Channels Digital Amp-Out CP
  Amp-Out caps: ofs=0x00, nsteps=0x00, stepsize=0x00, mute=1
  Amp-Out vals:  [0x00 0x00]
  Pincap 0x0b000094: OUT Detect HBR HDMI DP
  Pin Default 0x58560010: [N/A] Digital Out at Int HDMI
    Conn = Digital, Color = Unknown
    DefAssociation = 0x1, Sequence = 0x0
  Pin-ctls: 0x40: OUT
  Unsolicited: tag=00, enabled=0
  Power states:  D0 D3 EPSS
  Power: setting=D0, actual=D0
  Devices: 0
  Connection: 3
     0x02 0x03* 0x04

Those pins were filtered out by commit 116dcde63806 ("ALSA: HDA: Remove
unconnected PCM devices for Intel HDMI"). However, jacks that support
detection won't have the issues the commit addresses.

So still add the pin if it supports jack detection.

Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
---
 sound/pci/hda/patch_hdmi.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Takashi Iwai Aug. 4, 2020, 9:04 a.m. UTC | #1
On Tue, 04 Aug 2020 09:29:25 +0200,
Kai-Heng Feng wrote:
> 
> HDMI on some platforms doesn't enable audio support because its Port
> Connectivity [31:30] is set to AC_JACK_PORT_NONE:
> Node 0x05 [Pin Complex] wcaps 0x40778d: 8-Channels Digital Amp-Out CP
>   Amp-Out caps: ofs=0x00, nsteps=0x00, stepsize=0x00, mute=1
>   Amp-Out vals:  [0x00 0x00]
>   Pincap 0x0b000094: OUT Detect HBR HDMI DP
>   Pin Default 0x58560010: [N/A] Digital Out at Int HDMI
>     Conn = Digital, Color = Unknown
>     DefAssociation = 0x1, Sequence = 0x0
>   Pin-ctls: 0x40: OUT
>   Unsolicited: tag=00, enabled=0
>   Power states:  D0 D3 EPSS
>   Power: setting=D0, actual=D0
>   Devices: 0
>   Connection: 3
>      0x02 0x03* 0x04
> 
> Those pins were filtered out by commit 116dcde63806 ("ALSA: HDA: Remove
> unconnected PCM devices for Intel HDMI"). However, jacks that support
> detection won't have the issues the commit addresses.
> 
> So still add the pin if it supports jack detection.
> 
> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>

Which platform did show the problem?

I'm reluctant to apply this change as it would potentially break the
existing system.  If we must to apply, maybe it's safer to apply it
conditionally to the limited devices.


thanks,

Takashi

> ---
>  sound/pci/hda/patch_hdmi.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
> index cd46247988e4..db3a5148bd40 100644
> --- a/sound/pci/hda/patch_hdmi.c
> +++ b/sound/pci/hda/patch_hdmi.c
> @@ -1701,7 +1701,8 @@ static int hdmi_add_pin(struct hda_codec *codec, hda_nid_t pin_nid)
>  	 * all device entries on the same pin
>  	 */
>  	config = snd_hda_codec_get_pincfg(codec, pin_nid);
> -	if (get_defcfg_connect(config) == AC_JACK_PORT_NONE)
> +	if ((get_defcfg_connect(config) == AC_JACK_PORT_NONE) &&
> +	    !(caps & AC_PINCAP_PRES_DETECT))
>  		return 0;
>  
>  	/*
> -- 
> 2.17.1
>
Kai-Heng Feng Aug. 4, 2020, 9:31 a.m. UTC | #2
> On Aug 4, 2020, at 17:04, Takashi Iwai <tiwai@suse.de> wrote:
> 
> On Tue, 04 Aug 2020 09:29:25 +0200,
> Kai-Heng Feng wrote:
>> 
>> HDMI on some platforms doesn't enable audio support because its Port
>> Connectivity [31:30] is set to AC_JACK_PORT_NONE:
>> Node 0x05 [Pin Complex] wcaps 0x40778d: 8-Channels Digital Amp-Out CP
>>  Amp-Out caps: ofs=0x00, nsteps=0x00, stepsize=0x00, mute=1
>>  Amp-Out vals:  [0x00 0x00]
>>  Pincap 0x0b000094: OUT Detect HBR HDMI DP
>>  Pin Default 0x58560010: [N/A] Digital Out at Int HDMI
>>    Conn = Digital, Color = Unknown
>>    DefAssociation = 0x1, Sequence = 0x0
>>  Pin-ctls: 0x40: OUT
>>  Unsolicited: tag=00, enabled=0
>>  Power states:  D0 D3 EPSS
>>  Power: setting=D0, actual=D0
>>  Devices: 0
>>  Connection: 3
>>     0x02 0x03* 0x04
>> 
>> Those pins were filtered out by commit 116dcde63806 ("ALSA: HDA: Remove
>> unconnected PCM devices for Intel HDMI"). However, jacks that support
>> detection won't have the issues the commit addresses.
>> 
>> So still add the pin if it supports jack detection.
>> 
>> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> 
> Which platform did show the problem?

An HP desktop.

> 
> I'm reluctant to apply this change as it would potentially break the
> existing system.  If we must to apply, maybe it's safer to apply it
> conditionally to the limited devices.

Hmm, I find it's a bit hard to match a specific device, because the ID seems to be rather generic:
Codec: Intel Kabylake HDMI
Address: 2                      
AFG Function Id: 0x1 (unsol 0)
Vendor Id: 0x8086280b         
Subsystem Id: 0x80860101
Revision Id: 0x100000

Should we use DMI string instead?

Kai-Heng

> 
> 
> thanks,
> 
> Takashi
> 
>> ---
>> sound/pci/hda/patch_hdmi.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>> 
>> diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
>> index cd46247988e4..db3a5148bd40 100644
>> --- a/sound/pci/hda/patch_hdmi.c
>> +++ b/sound/pci/hda/patch_hdmi.c
>> @@ -1701,7 +1701,8 @@ static int hdmi_add_pin(struct hda_codec *codec, hda_nid_t pin_nid)
>> 	 * all device entries on the same pin
>> 	 */
>> 	config = snd_hda_codec_get_pincfg(codec, pin_nid);
>> -	if (get_defcfg_connect(config) == AC_JACK_PORT_NONE)
>> +	if ((get_defcfg_connect(config) == AC_JACK_PORT_NONE) &&
>> +	    !(caps & AC_PINCAP_PRES_DETECT))
>> 		return 0;
>> 
>> 	/*
>> -- 
>> 2.17.1
Takashi Iwai Aug. 4, 2020, 9:48 a.m. UTC | #3
On Tue, 04 Aug 2020 11:31:59 +0200,
Kai-Heng Feng wrote:
> 
> 
> 
> > On Aug 4, 2020, at 17:04, Takashi Iwai <tiwai@suse.de> wrote:
> > 
> > On Tue, 04 Aug 2020 09:29:25 +0200,
> > Kai-Heng Feng wrote:
> >> 
> >> HDMI on some platforms doesn't enable audio support because its Port
> >> Connectivity [31:30] is set to AC_JACK_PORT_NONE:
> >> Node 0x05 [Pin Complex] wcaps 0x40778d: 8-Channels Digital Amp-Out CP
> >>  Amp-Out caps: ofs=0x00, nsteps=0x00, stepsize=0x00, mute=1
> >>  Amp-Out vals:  [0x00 0x00]
> >>  Pincap 0x0b000094: OUT Detect HBR HDMI DP
> >>  Pin Default 0x58560010: [N/A] Digital Out at Int HDMI
> >>    Conn = Digital, Color = Unknown
> >>    DefAssociation = 0x1, Sequence = 0x0
> >>  Pin-ctls: 0x40: OUT
> >>  Unsolicited: tag=00, enabled=0
> >>  Power states:  D0 D3 EPSS
> >>  Power: setting=D0, actual=D0
> >>  Devices: 0
> >>  Connection: 3
> >>     0x02 0x03* 0x04
> >> 
> >> Those pins were filtered out by commit 116dcde63806 ("ALSA: HDA: Remove
> >> unconnected PCM devices for Intel HDMI"). However, jacks that support
> >> detection won't have the issues the commit addresses.
> >> 
> >> So still add the pin if it supports jack detection.
> >> 
> >> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> > 
> > Which platform did show the problem?
> 
> An HP desktop.

Well, what I meant was about which codec.  And now I see it in the
below.

> > I'm reluctant to apply this change as it would potentially break the
> > existing system.  If we must to apply, maybe it's safer to apply it
> > conditionally to the limited devices.
> 
> Hmm, I find it's a bit hard to match a specific device, because the ID seems to be rather generic:
> Codec: Intel Kabylake HDMI
> Address: 2                      
> AFG Function Id: 0x1 (unsol 0)
> Vendor Id: 0x8086280b         
> Subsystem Id: 0x80860101
> Revision Id: 0x100000
> 
> Should we use DMI string instead?

So it's a Kabylake, and I presume that it's rather an old machine.
Is this for docking station or anything else?

Basically the pin capability is rather fixed by the chip design while
the pin configuration is set by BIOS.  And we follow the BIOS setup
for determining which pins are actually alive.  That said, the bug is
a BIOS bug.

Note that PCI SSID bound with this codec might have a different
number, so we might be still able to use the standard quirk table to
pick up.


thanks,

Takashi

> > 
> > thanks,
> > 
> > Takashi
> > 
> >> ---
> >> sound/pci/hda/patch_hdmi.c | 3 ++-
> >> 1 file changed, 2 insertions(+), 1 deletion(-)
> >> 
> >> diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
> >> index cd46247988e4..db3a5148bd40 100644
> >> --- a/sound/pci/hda/patch_hdmi.c
> >> +++ b/sound/pci/hda/patch_hdmi.c
> >> @@ -1701,7 +1701,8 @@ static int hdmi_add_pin(struct hda_codec *codec, hda_nid_t pin_nid)
> >> 	 * all device entries on the same pin
> >> 	 */
> >> 	config = snd_hda_codec_get_pincfg(codec, pin_nid);
> >> -	if (get_defcfg_connect(config) == AC_JACK_PORT_NONE)
> >> +	if ((get_defcfg_connect(config) == AC_JACK_PORT_NONE) &&
> >> +	    !(caps & AC_PINCAP_PRES_DETECT))
> >> 		return 0;
> >> 
> >> 	/*
> >> -- 
> >> 2.17.1
>
Kai-Heng Feng Aug. 4, 2020, 11:42 a.m. UTC | #4
> On Aug 4, 2020, at 17:48, Takashi Iwai <tiwai@suse.de> wrote:
> 
> On Tue, 04 Aug 2020 11:31:59 +0200,
> Kai-Heng Feng wrote:
>> 
>> 
>> 
>>> On Aug 4, 2020, at 17:04, Takashi Iwai <tiwai@suse.de> wrote:
>>> 
>>> On Tue, 04 Aug 2020 09:29:25 +0200,
>>> Kai-Heng Feng wrote:
>>>> 
>>>> HDMI on some platforms doesn't enable audio support because its Port
>>>> Connectivity [31:30] is set to AC_JACK_PORT_NONE:
>>>> Node 0x05 [Pin Complex] wcaps 0x40778d: 8-Channels Digital Amp-Out CP
>>>> Amp-Out caps: ofs=0x00, nsteps=0x00, stepsize=0x00, mute=1
>>>> Amp-Out vals:  [0x00 0x00]
>>>> Pincap 0x0b000094: OUT Detect HBR HDMI DP
>>>> Pin Default 0x58560010: [N/A] Digital Out at Int HDMI
>>>>   Conn = Digital, Color = Unknown
>>>>   DefAssociation = 0x1, Sequence = 0x0
>>>> Pin-ctls: 0x40: OUT
>>>> Unsolicited: tag=00, enabled=0
>>>> Power states:  D0 D3 EPSS
>>>> Power: setting=D0, actual=D0
>>>> Devices: 0
>>>> Connection: 3
>>>>    0x02 0x03* 0x04
>>>> 
>>>> Those pins were filtered out by commit 116dcde63806 ("ALSA: HDA: Remove
>>>> unconnected PCM devices for Intel HDMI"). However, jacks that support
>>>> detection won't have the issues the commit addresses.
>>>> 
>>>> So still add the pin if it supports jack detection.
>>>> 
>>>> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
>>> 
>>> Which platform did show the problem?
>> 
>> An HP desktop.
> 
> Well, what I meant was about which codec.  And now I see it in the
> below.
> 
>>> I'm reluctant to apply this change as it would potentially break the
>>> existing system.  If we must to apply, maybe it's safer to apply it
>>> conditionally to the limited devices.
>> 
>> Hmm, I find it's a bit hard to match a specific device, because the ID seems to be rather generic:
>> Codec: Intel Kabylake HDMI
>> Address: 2                      
>> AFG Function Id: 0x1 (unsol 0)
>> Vendor Id: 0x8086280b         
>> Subsystem Id: 0x80860101
>> Revision Id: 0x100000
>> 
>> Should we use DMI string instead?
> 
> So it's a Kabylake, and I presume that it's rather an old machine.
> Is this for docking station or anything else?

The system is Comet Lake CPU so it's fairly recent. I think most Comet Lake platforms still use "Kabylake HDMI".

> 
> Basically the pin capability is rather fixed by the chip design while
> the pin configuration is set by BIOS.  And we follow the BIOS setup
> for determining which pins are actually alive.  That said, the bug is
> a BIOS bug.

Yes but sometimes vendors are reluctant to fix it because "Windows doesn't have this issue".

> 
> Note that PCI SSID bound with this codec might have a different
> number, so we might be still able to use the standard quirk table to
> pick up.

Ok. Will send v2 with a quirk list.

Kai-Heng

> 
> 
> thanks,
> 
> Takashi
> 
>>> 
>>> thanks,
>>> 
>>> Takashi
>>> 
>>>> ---
>>>> sound/pci/hda/patch_hdmi.c | 3 ++-
>>>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>>> 
>>>> diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
>>>> index cd46247988e4..db3a5148bd40 100644
>>>> --- a/sound/pci/hda/patch_hdmi.c
>>>> +++ b/sound/pci/hda/patch_hdmi.c
>>>> @@ -1701,7 +1701,8 @@ static int hdmi_add_pin(struct hda_codec *codec, hda_nid_t pin_nid)
>>>> 	 * all device entries on the same pin
>>>> 	 */
>>>> 	config = snd_hda_codec_get_pincfg(codec, pin_nid);
>>>> -	if (get_defcfg_connect(config) == AC_JACK_PORT_NONE)
>>>> +	if ((get_defcfg_connect(config) == AC_JACK_PORT_NONE) &&
>>>> +	    !(caps & AC_PINCAP_PRES_DETECT))
>>>> 		return 0;
>>>> 
>>>> 	/*
>>>> -- 
>>>> 2.17.1
Takashi Iwai Aug. 4, 2020, 11:48 a.m. UTC | #5
On Tue, 04 Aug 2020 13:42:03 +0200,
Kai-Heng Feng wrote:
> 
> 
> 
> > On Aug 4, 2020, at 17:48, Takashi Iwai <tiwai@suse.de> wrote:
> > 
> > On Tue, 04 Aug 2020 11:31:59 +0200,
> > Kai-Heng Feng wrote:
> >> 
> >> 
> >> 
> >>> On Aug 4, 2020, at 17:04, Takashi Iwai <tiwai@suse.de> wrote:
> >>> 
> >>> On Tue, 04 Aug 2020 09:29:25 +0200,
> >>> Kai-Heng Feng wrote:
> >>>> 
> >>>> HDMI on some platforms doesn't enable audio support because its Port
> >>>> Connectivity [31:30] is set to AC_JACK_PORT_NONE:
> >>>> Node 0x05 [Pin Complex] wcaps 0x40778d: 8-Channels Digital Amp-Out CP
> >>>> Amp-Out caps: ofs=0x00, nsteps=0x00, stepsize=0x00, mute=1
> >>>> Amp-Out vals:  [0x00 0x00]
> >>>> Pincap 0x0b000094: OUT Detect HBR HDMI DP
> >>>> Pin Default 0x58560010: [N/A] Digital Out at Int HDMI
> >>>>   Conn = Digital, Color = Unknown
> >>>>   DefAssociation = 0x1, Sequence = 0x0
> >>>> Pin-ctls: 0x40: OUT
> >>>> Unsolicited: tag=00, enabled=0
> >>>> Power states:  D0 D3 EPSS
> >>>> Power: setting=D0, actual=D0
> >>>> Devices: 0
> >>>> Connection: 3
> >>>>    0x02 0x03* 0x04
> >>>> 
> >>>> Those pins were filtered out by commit 116dcde63806 ("ALSA: HDA: Remove
> >>>> unconnected PCM devices for Intel HDMI"). However, jacks that support
> >>>> detection won't have the issues the commit addresses.
> >>>> 
> >>>> So still add the pin if it supports jack detection.
> >>>> 
> >>>> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> >>> 
> >>> Which platform did show the problem?
> >> 
> >> An HP desktop.
> > 
> > Well, what I meant was about which codec.  And now I see it in the
> > below.
> > 
> >>> I'm reluctant to apply this change as it would potentially break the
> >>> existing system.  If we must to apply, maybe it's safer to apply it
> >>> conditionally to the limited devices.
> >> 
> >> Hmm, I find it's a bit hard to match a specific device, because the ID seems to be rather generic:
> >> Codec: Intel Kabylake HDMI
> >> Address: 2                      
> >> AFG Function Id: 0x1 (unsol 0)
> >> Vendor Id: 0x8086280b         
> >> Subsystem Id: 0x80860101
> >> Revision Id: 0x100000
> >> 
> >> Should we use DMI string instead?
> > 
> > So it's a Kabylake, and I presume that it's rather an old machine.
> > Is this for docking station or anything else?
> 
> The system is Comet Lake CPU so it's fairly recent. I think most Comet Lake platforms still use "Kabylake HDMI".

OK.

> > Basically the pin capability is rather fixed by the chip design while
> > the pin configuration is set by BIOS.  And we follow the BIOS setup
> > for determining which pins are actually alive.  That said, the bug is
> > a BIOS bug.
> 
> Yes but sometimes vendors are reluctant to fix it because "Windows doesn't have this issue".

Yes, and BIOS is always buggy, it's a fundamental rule of the world :)
So I'm not against adding the workaround itself.  The problem is that
applying the workaround globally would cause another regression.

> > Note that PCI SSID bound with this codec might have a different
> > number, so we might be still able to use the standard quirk table to
> > pick up.
> 
> Ok. Will send v2 with a quirk list.

Thanks.  We can reconsider to widen the application of the workaround
once when we see the requirement (i.e. more machines with buggy BIOS
setup.)


Takashi


> Kai-Heng
> 
> > 
> > 
> > thanks,
> > 
> > Takashi
> > 
> >>> 
> >>> thanks,
> >>> 
> >>> Takashi
> >>> 
> >>>> ---
> >>>> sound/pci/hda/patch_hdmi.c | 3 ++-
> >>>> 1 file changed, 2 insertions(+), 1 deletion(-)
> >>>> 
> >>>> diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
> >>>> index cd46247988e4..db3a5148bd40 100644
> >>>> --- a/sound/pci/hda/patch_hdmi.c
> >>>> +++ b/sound/pci/hda/patch_hdmi.c
> >>>> @@ -1701,7 +1701,8 @@ static int hdmi_add_pin(struct hda_codec *codec, hda_nid_t pin_nid)
> >>>> 	 * all device entries on the same pin
> >>>> 	 */
> >>>> 	config = snd_hda_codec_get_pincfg(codec, pin_nid);
> >>>> -	if (get_defcfg_connect(config) == AC_JACK_PORT_NONE)
> >>>> +	if ((get_defcfg_connect(config) == AC_JACK_PORT_NONE) &&
> >>>> +	    !(caps & AC_PINCAP_PRES_DETECT))
> >>>> 		return 0;
> >>>> 
> >>>> 	/*
> >>>> -- 
> >>>> 2.17.1
>
diff mbox series

Patch

diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
index cd46247988e4..db3a5148bd40 100644
--- a/sound/pci/hda/patch_hdmi.c
+++ b/sound/pci/hda/patch_hdmi.c
@@ -1701,7 +1701,8 @@  static int hdmi_add_pin(struct hda_codec *codec, hda_nid_t pin_nid)
 	 * all device entries on the same pin
 	 */
 	config = snd_hda_codec_get_pincfg(codec, pin_nid);
-	if (get_defcfg_connect(config) == AC_JACK_PORT_NONE)
+	if ((get_defcfg_connect(config) == AC_JACK_PORT_NONE) &&
+	    !(caps & AC_PINCAP_PRES_DETECT))
 		return 0;
 
 	/*