Re: [PATCH] rtw88: 8821c: disable the ASPM of RTL8821CE

Pkshih [off-list ref] 於 2021年12月11日 週六 下午2:31寫道：

quoted

-----Original Message-----
From: Jian-Hong Pan <redacted>
Sent: Friday, December 10, 2021 5:34 PM
To: Kai-Heng Feng <redacted>
Cc: Pkshih <pkshih@realtek.com>; Yan-Hsuan Chuang <redacted>; Kalle Valo
[off-list ref]; linux-wireless@vger.kernel.org; netdev@vger.kernel.org;
linux-kernel@vger.kernel.org; linux@endlessos.org
Subject: Re: [PATCH] rtw88: 8821c: disable the ASPM of RTL8821CE

Kai-Heng Feng [off-list ref] 於 2021年12月10日 週五 下午5:24寫道：

quoted

On Fri, Dec 10, 2021 at 5:00 PM Pkshih [off-list ref] wrote:

quoted

+Kai-Heng

quoted

-----Original Message-----
From: Jian-Hong Pan <redacted>
Sent: Friday, December 10, 2021 4:17 PM
To: Pkshih <pkshih@realtek.com>; Yan-Hsuan Chuang <redacted>; Kalle Valo
[off-list ref]
Cc: linux-wireless@vger.kernel.org; netdev@vger.kernel.org; linux-kernel@vger.kernel.org;
linux@endlessos.org; Jian-Hong Pan [off-list ref]
Subject: [PATCH] rtw88: 8821c: disable the ASPM of RTL8821CE

More and more laptops become frozen, due to the equipped RTL8821CE.

This patch follows the idea mentioned in commits 956c6d4f20c5 ("rtw88:
add quirks to disable pci capabilities") and 1d4dcaf3db9bd ("rtw88: add
quirk to disable pci caps on HP Pavilion 14-ce0xxx"), but disables its
PCI ASPM capability of RTL8821CE directly, instead of checking DMI.

Buglink:https://bugzilla.kernel.org/show_bug.cgi?id=215239
Fixes: 1d4dcaf3db9bd ("rtw88: add quirk to disable pci caps on HP Pavilion 14-ce0xxx")
Signed-off-by: Jian-Hong Pan <redacted>

We also discuss similar thing in this thread:
https://bugzilla.kernel.org/show_bug.cgi?id=215131

Since we still want to turn on ASPM to save more power, I would like to
enumerate the blacklist. Does it work to you?

Too many platforms are affected, the blacklist method won't scale.

Exactly!

Got it.

quoted

quoted

Right now it seems like only Intel platforms are affected, so can I
propose a patch to disable ASPM when its upstream port is Intel?

I only have laptops with Intel chip now.  So, I am not sure the status
with AMD platforms.
If this is true, then "disable ASPM when its upstream port is Intel"
might be a good idea.

Jian-Hong, could you try Kai-Heng's workaround that only turn off ASPM
during NAPI poll function. If it also works to you, I think it is okay
to apply this workaround to all Intel platform with RTL8821CE chipset.
Because this workaround has little (almost no) impact of power consumption.

According to Kai-Heng's hack patch [1] and the comment [2] mentioning
checking "ref_cnt" by rtw_pci_link_ps(), I arrange the patch as
following.
This patch only disables ASPM (if the hardware has the capability)
when system gets into rtw_pci_napi_poll() and re-enables ASPM when it
leaves rtw_pci_napi_poll().  It is as Ping-Ke mentioned "only turn off
ASPM during NAPI poll function".
The WiFi & BT work, and system is still alive after I use the internet
awhile.  Besides, there is no more "pci bus timeout, check dma status"
error.

[1] https://bugzilla.kernel.org/show_bug.cgi?id=215131#c11
[2] https://bugzilla.kernel.org/show_bug.cgi?id=215131#c15

Jian-Hong Pan

diff --git a/drivers/net/wireless/realtek/rtw88/pci.c

b/drivers/net/wireless/realtek/rtw88/pci.c
index a7a6ebfaa203..a6fdddecd37d 100644

--- a/drivers/net/wireless/realtek/rtw88/pci.c
+++ b/drivers/net/wireless/realtek/rtw88/pci.c

@@ -1658,6 +1658,7 @@ static int rtw_pci_napi_poll(struct napi_struct

*napi, int budget)
                                              priv);
        int work_done = 0;

+       rtw_pci_link_ps(rtwdev, false);
        while (work_done < budget) {
                u32 work_done_once;

@@ -1681,6 +1682,7 @@ static int rtw_pci_napi_poll(struct napi_struct

*napi, int budget)
                if (rtw_pci_get_hw_rx_ring_nr(rtwdev, rtwpci))
                        napi_schedule(napi);
        }
+       rtw_pci_link_ps(rtwdev, true);

        return work_done;
 }

quoted

quoted

quoted

If so, please help to add one quirk entry of your platform.

Another thing is that "attachment 299735" is another workaround for certain
platform. And, we plan to add quirk to enable this workaround.
Could you try if it works to you?

When the hardware is doing DMA, it should initiate leaving ASPM L1,
correct? So in theory my workaround should be benign enough for most
platforms.

I don't see and know the detail of hardware waveform, but I think your
understanding is correct.

--
Ping-Ke