Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Sat, 17 Sep 2022 07:51:24 +0200
Message-ID: <87k062r1k3.wl-tiwai@suse.de>
From:   Takashi Iwai <tiwai@suse.de>
To:     "Arava, Jairaj" <jairaj.arava@intel.com>
Cc:     Takashi Iwai <tiwai@suse.de>, Pshou <pshou@realtek.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "Nujella, Sathyanarayana" <sathyanarayana.nujella@intel.com>,
        "Prabhu, Swarna" <swarna.prabhu@intel.com>,
        "Afzal, Naeem M" <naeem.m.afzal@intel.com>,
        "Hsu, Shui-Wen" <swhsu4021@realtek.com>,
        "Perati, RK" <rk.perati@intel.com>,
        "Mandri, Padmashree" <padmashree.mandri@intel.com>,
        Kailang <kailang@realtek.com>
Subject: Re: Sarien/Dorset device: After system resumed from suspend, 3.5m jack is still shown as detected when unplugged during suspend 
In-Reply-To: <DM4PR11MB53570968A455F76371004B689F489@DM4PR11MB5357.namprd11.prod.outlook.com>
References: <ee87941f7f654213924aa1bc79db32f7@realtek.com>
        <874jx7vdhs.wl-tiwai@suse.de>
        <DM4PR11MB53570968A455F76371004B689F489@DM4PR11MB5357.namprd11.prod.outlook.com>
User-Agent: Wanderlust/2.15.9 (Almost Unreal) Emacs/27.2 Mule/6.0
MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue")
Content-Type: text/plain; charset=US-ASCII
Precedence: bulk

On Fri, 16 Sep 2022 21:50:51 +0200,
Arava, Jairaj wrote:
> 
> Hi Takashi,
> 
> This discussion is regarding the bug https://bugzilla.kernel.org/show_bug.cgi?id=215297.

Did you read the comment 17?  You haven't analyzed yet properly what
was going wrong.

> As we know https://lore.kernel.org/alsa-devel/20210310112809.9215-3-tiwai@suse.de/ is causing the 3.5mm headset jack detection issue during system suspend resume. So after reverting this patch the issue is not seen since the unsol event is handled without this patch.
> 
> Since we  see in https://elixir.bootlin.com/linux/latest/source/sound/pci/hda/patch_realtek.c#L992 the codec driver has  snd_hda_jack_unsol_event as the unsol event handler. Hence after reverting your patch, from https://elixir.bootlin.com/linux/latest/source/sound/pci/hda/hda_bind.c#L56  the driver is calling https://elixir.bootlin.com/linux/latest/source/sound/pci/hda/hda_jack.c#L713 to handle the unsolv event.
> 
> Based on above observance, seems snd_hda_jack_unsol_event is mandatory for the intel platform Realtek codec driver to handle such unsolv enets.  Hence, @Pshou added the PM flag check as the patch was tested by Nvidia.

If the hardware can't detect the jack at the resume by some reason,
that must be the cause, and not because it doesn't handle the unsol
event during the suspend.

Please try to debug more deeply at first.


thanks,

Takashi

> 
> Thanks,
> Jai 
> -----Original Message-----
> From: Takashi Iwai <tiwai@suse.de> 
> Sent: Friday, September 16, 2022 3:08 AM
> To: Pshou <pshou@realtek.com>
> Cc: linux-kernel@vger.kernel.org; Takashi Iwai <tiwai@suse.de>; Arava, Jairaj <jairaj.arava@intel.com>; Nujella, Sathyanarayana <sathyanarayana.nujella@intel.com>; Prabhu, Swarna <swarna.prabhu@intel.com>; Afzal, Naeem M <naeem.m.afzal@intel.com>; Hsu, Shui-Wen <swhsu4021@realtek.com>; Perati, RK <rk.perati@intel.com>; Mandri, Padmashree <padmashree.mandri@intel.com>; Kailang <kailang@realtek.com>
> Subject: Re: Sarien/Dorset device: After system resumed from suspend, 3.5m jack is still shown as detected when unplugged during suspend 
> 
> On Fri, 16 Sep 2022 07:34:38 +0200,
> Pshou wrote:
> > 
> > 
> > Hi Takashi Iwai:
> > 
> > Can you help me update this PATCH file?
> > 
> > Check if ignore unsol events duing system suspend/resume and NVIDIA 
> > chip in hda_codec_unsol_event().
> > 
> > Signed-off-by:PeiSen Hou<pshou@realtek.com>
> > 
> > Signed-off-by: Jairaj Arava <jairaj.arava@intel.com>
> > 
> > diff --git a/sound/pci/hda/hda_bind.c b/sound/pci/hda/hda_bind.c
> > 
> > index 1a868dd9dc4b..75560ff6eb83 100644
> > 
> > --- a/sound/pci/hda/hda_bind.c
> > 
> > +++ b/sound/pci/hda/hda_bind.c
> > 
> > @@ -50,7 +50,8 @@ static void hda_codec_unsol_event(struct hdac_device 
> > *dev, unsigned int ev)
> > 
> >        /* ignore unsol events during system suspend/resume */
> > 
> >       if (codec->core.dev.power.power_state.event != PM_EVENT_ON)
> > 
> > -               return;
> > 
> > +              if (codec->core.vendor_id == PCI_VENDOR_ID_NVIDIA)
> > 
> > +                      return;
> > 
> >        if (codec->patch_ops.unsol_event)
> > 
> >               codec->patch_ops.unsol_event(codec, ev);
> 
> Hmm, this doesn't look safe.  We also want to avoid the unsol event handling during the PM state transition, too.  So, if any, this should be allowed only at PM_EVENT_SUSPEND or PM_EVENT_HIBERNATE.
> 
> Also, checking the codec vendor ID here is no good way.  We may add a new flag for the special behavior (either allowing the unsol handling or prohibiting).
> 
> But, from your patch, I don't see any reason *why* this has to be changed in that way.  Could you give more backgrounds?
> 
> 
> thanks,
> 
> Takashi
>