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@xxxxxxx> 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@xxxxxxx/ 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@xxxxxxx> 
>> Sent: Friday, September 16, 2022 3:08 AM
>> To: Pshou <pshou@xxxxxxxxxxx>
>> Cc: linux-kernel@xxxxxxxxxxxxxxx; Takashi Iwai <tiwai@xxxxxxx>; Arava, Jairaj <jairaj.arava@xxxxxxxxx>; Nujella, Sathyanarayana <sathyanarayana.nujella@xxxxxxxxx>; Prabhu, Swarna <swarna.prabhu@xxxxxxxxx>; Afzal, Naeem M <naeem.m.afzal@xxxxxxxxx>; Hsu, Shui-Wen <swhsu4021@xxxxxxxxxxx>; Perati, RK <rk.perati@xxxxxxxxx>; Mandri, Padmashree <padmashree.mandri@xxxxxxxxx>; Kailang <kailang@xxxxxxxxxxx>
>> 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@xxxxxxxxxxx>
>>> 
>>> Signed-off-by: Jairaj Arava <jairaj.arava@xxxxxxxxx>
>>> 
>>> 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
>>