Re: [PATCH] ALSA: hda/hdmi: Add pins with jack detection support

[Takashi Iwai <tiwai@suse.de>]

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
>