RE: [BUG] wifi: rtw88: Hard system freeze on RTL8821CE when power_save is enabled (LPS/ASPM conflict)

[Ping-Ke Shih]

LB F <goainwo@xxxxxxxxx> wrote:
> Ping-Ke Shih <pkshih@xxxxxxxxxxx> wrote:
> > I'd adopt your suggestion (dynamic LPS_DEEP_MODE_NONE) if the test
> > is positive.
> 
> Hi Ping-Ke,
> 
> Following your suggestion, I performed an additional experiment to
> validate the dynamic LPS_DEEP_MODE_NONE idea. Please treat this
> purely as a field test report -- I am not a kernel developer, and the
> implementation below is certainly not upstream-quality. I am sharing
> it only in the hope that it helps you design a proper solution.
> 
> What I did:
> 
> I extended your DMI quirk in pci.c with an additional capability flag
> for LPS Deep mode. The only file touched was pci.c (your patch) --
> main.c was left completely unmodified.
> 
> The changes to your patch are as follows:
> 
>   /* 1. Extended the capabilities enum */
>   enum rtw88_quirk_dis_pci_caps {
>           QUIRK_DIS_PCI_CAP_ASPM,
>           QUIRK_DIS_PCI_CAP_LPS_DEEP,  /* test addition */
>   };
> 
>   /* 2. Extended disable_pci_caps() callback */
>   static int disable_pci_caps(const struct dmi_system_id *dmi)
>   {
>           uintptr_t dis_caps = (uintptr_t)dmi->driver_data;
> 
>           if (dis_caps & BIT(QUIRK_DIS_PCI_CAP_ASPM))
>                   rtw_pci_disable_aspm = true;
> 
>           if (dis_caps & BIT(QUIRK_DIS_PCI_CAP_LPS_DEEP))
>                   rtw_disable_lps_deep_mode = true;
> 
>           return 1;
>   }
> 
>   /* 3. Both flags set for the HP P3S95EA#ACB entry */
>   .driver_data = (void *)(BIT(QUIRK_DIS_PCI_CAP_ASPM) |
>                           BIT(QUIRK_DIS_PCI_CAP_LPS_DEEP)),
> 
> I am aware that setting rtw_disable_lps_deep_mode from pci.c is
> architecturally impure -- it is a global flag that would affect all
> rtw88 devices in a hypothetical multi-adapter system. A proper
> per-device solution (e.g. a flag inside struct rtw_dev set during
> probe) would be cleaner. I simply used the existing global as the
> most straightforward way to validate the concept.
> 
> Verification:
> 
> Confirmed no rtw88-related entries exist in /etc/modprobe.d/,
> /lib/modprobe.d/, or /run/modprobe.d/, ruling out any external
> parameter injection.
> 
> After loading the patched modules, the following was confirmed via
> sysfs:
> 
>   /sys/module/rtw88_core/parameters/disable_lps_deep_mode = Y
>   /sys/module/rtw88_pci/parameters/disable_aspm = Y
> 
> This confirms the DMI quirk is the sole source of both values.
> 
> Results (10-minute idle observation, battery power, wifi.powersave=3):
> 
>   With your ASPM patch alone (LPS Deep still active):
>     - periodic "failed to send h2c command" bursts observed
>     - occasional WiFi throughput drops and Bluetooth audio stuttering
> 
>   With ASPM patch + LPS Deep disabled via the quirk:
>     - h2c=0, lps=0 across the entire observation window
>     - WiFi throughput stable, Bluetooth audio uninterrupted
> 
> The result confirms that disabling LPS Deep Mode in addition to ASPM
> completely eliminates the remaining firmware timeout loop on this
> platform.
> 
> I hope this experiment is useful as a data point. Please feel free to
> discard the implementation and design a proper solution -- I am ready
> to test any updated patch you send.

Thanks for your analysis of TX/RX paths, and the changes above and
verifications. :)

I'd update the patch as your proposal and send a patch. For suggestions of
TX/RX paths, I only read them a little bit, and I will study them entirely
when I have more free time. 

Ping-Ke