Re: Sarien/Dorset device: After system resumed from suspend, 3.5m jack is still shown as detected when unplugged during suspend

Re: Sarien/Dorset device: After system resumed from suspend, 3.5m jack is still shown as detected when unplugged during suspend

[Takashi Iwai]

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

RE: Sarien/Dorset device: After system resumed from suspend, 3.5m jack is still shown as detected when unplugged during suspend

[Arava, Jairaj]

Hi Takashi,

This discussion is regarding the bug https://bugzilla.kernel.org/show_bug.cgi?id=215297.

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.

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

Re: Sarien/Dorset device: After system resumed from suspend, 3.5m jack is still shown as detected when unplugged during suspend

[Takashi Iwai]

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
> 

Re: Sarien/Dorset device: After system resumed from suspend, 3.5m jack is still shown as detected when unplugged during suspend

[Arava, Jairaj]

Hi Takashi,

After reading the comment#17, we started looking from codec end and we see the unsol event handler is not invoked to handle it & thought it is mandate for the codec.
 However as said, by will look further deep into it.

@Pshou,
Want to get undersrand from the codec end further. How can we expect the codec to behave for such events during suspend & resume?

Thanks,
Jai

> On Sep 16, 2022, at 10:51 PM, Takashi Iwai <tiwai@suse.de> wrote:
> 
> 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
>>