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

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

[Jian-Hong Pan]

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 <jhp@endlessos.org>
---
 drivers/net/wireless/realtek/rtw88/main.h     |  3 ++
 drivers/net/wireless/realtek/rtw88/pci.c      | 33 ++-----------------
 drivers/net/wireless/realtek/rtw88/pci.h      |  5 +++
 drivers/net/wireless/realtek/rtw88/rtw8821c.c |  3 ++
 4 files changed, 14 insertions(+), 30 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtw88/main.h b/drivers/net/wireless/realtek/rtw88/main.h
index bbdd535b64e7..31cd427a0949 100644
--- a/drivers/net/wireless/realtek/rtw88/main.h
+++ b/drivers/net/wireless/realtek/rtw88/main.h
@@ -1259,6 +1259,9 @@ struct rtw_chip_info {
 	const struct rtw_hw_reg *btg_reg;
 	const struct rtw_reg_domain *coex_info_hw_regs;
 	u32 wl_fw_desired_ver;
+
+	/* quirk flags */
+	u32 pci_quirk_data;
 };
 
 enum rtw_coex_bt_state_cnt {
diff --git a/drivers/net/wireless/realtek/rtw88/pci.c b/drivers/net/wireless/realtek/rtw88/pci.c
index a7a6ebfaa203..0a858db2d515 100644
--- a/drivers/net/wireless/realtek/rtw88/pci.c
+++ b/drivers/net/wireless/realtek/rtw88/pci.c
@@ -1702,14 +1702,9 @@ static void rtw_pci_napi_deinit(struct rtw_dev *rtwdev)
 	netif_napi_del(&rtwpci->napi);
 }
 
-enum rtw88_quirk_dis_pci_caps {
-	QUIRK_DIS_PCI_CAP_MSI,
-	QUIRK_DIS_PCI_CAP_ASPM,
-};
-
-static int disable_pci_caps(const struct dmi_system_id *dmi)
+static int disable_pci_caps_by_chip(const struct rtw_chip_info *chip)
 {
-	uintptr_t dis_caps = (uintptr_t)dmi->driver_data;
+	u32 dis_caps = chip->pci_quirk_data;
 
 	if (dis_caps & BIT(QUIRK_DIS_PCI_CAP_MSI))
 		rtw_disable_msi = true;
@@ -1719,28 +1714,6 @@ static int disable_pci_caps(const struct dmi_system_id *dmi)
 	return 1;
 }
 
-static const struct dmi_system_id rtw88_pci_quirks[] = {
-	{
-		.callback = disable_pci_caps,
-		.ident = "Protempo Ltd L116HTN6SPW",
-		.matches = {
-			DMI_MATCH(DMI_SYS_VENDOR, "Protempo Ltd"),
-			DMI_MATCH(DMI_PRODUCT_NAME, "L116HTN6SPW"),
-		},
-		.driver_data = (void *)BIT(QUIRK_DIS_PCI_CAP_ASPM),
-	},
-	{
-		.callback = disable_pci_caps,
-		.ident = "HP HP Pavilion Laptop 14-ce0xxx",
-		.matches = {
-			DMI_MATCH(DMI_SYS_VENDOR, "HP"),
-			DMI_MATCH(DMI_PRODUCT_NAME, "HP Pavilion Laptop 14-ce0xxx"),
-		},
-		.driver_data = (void *)BIT(QUIRK_DIS_PCI_CAP_ASPM),
-	},
-	{}
-};
-
 int rtw_pci_probe(struct pci_dev *pdev,
 		  const struct pci_device_id *id)
 {
@@ -1791,7 +1764,7 @@ int rtw_pci_probe(struct pci_dev *pdev,
 		goto err_destroy_pci;
 	}
 
-	dmi_check_system(rtw88_pci_quirks);
+	disable_pci_caps_by_chip(rtwdev->chip);
 	rtw_pci_phy_cfg(rtwdev);
 
 	ret = rtw_register_hw(rtwdev, hw);
diff --git a/drivers/net/wireless/realtek/rtw88/pci.h b/drivers/net/wireless/realtek/rtw88/pci.h
index 66f78eb7757c..f470387fbb9a 100644
--- a/drivers/net/wireless/realtek/rtw88/pci.h
+++ b/drivers/net/wireless/realtek/rtw88/pci.h
@@ -274,4 +274,9 @@ struct rtw_pci_tx_buffer_desc *get_tx_buffer_desc(struct rtw_pci_tx_ring *ring,
 	return (struct rtw_pci_tx_buffer_desc *)buf_desc;
 }
 
+enum rtw88_quirk_dis_pci_caps {
+	QUIRK_DIS_PCI_CAP_MSI,
+	QUIRK_DIS_PCI_CAP_ASPM,
+};
+
 #endif
diff --git a/drivers/net/wireless/realtek/rtw88/rtw8821c.c b/drivers/net/wireless/realtek/rtw88/rtw8821c.c
index 80a6f4da6acd..4d684534fa1e 100644
--- a/drivers/net/wireless/realtek/rtw88/rtw8821c.c
+++ b/drivers/net/wireless/realtek/rtw88/rtw8821c.c
@@ -15,6 +15,7 @@
 #include "debug.h"
 #include "bf.h"
 #include "regd.h"
+#include "pci.h"
 
 static const s8 lna_gain_table_0[8] = {22, 8, -6, -22, -31, -40, -46, -52};
 static const s8 lna_gain_table_1[16] = {10, 6, 2, -2, -6, -10, -14, -17,
@@ -1947,6 +1948,8 @@ struct rtw_chip_info rtw8821c_hw_spec = {
 
 	.coex_info_hw_regs_num = ARRAY_SIZE(coex_info_hw_regs_8821c),
 	.coex_info_hw_regs = coex_info_hw_regs_8821c,
+
+	.pci_quirk_data = BIT(QUIRK_DIS_PCI_CAP_ASPM),
 };
 EXPORT_SYMBOL(rtw8821c_hw_spec);
 
-- 
2.34.1

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

[Pkshih]

+Kai-Heng

> -----Original Message-----
> From: Jian-Hong Pan <jhp@endlessos.org>
> Sent: Friday, December 10, 2021 4:17 PM
> To: Pkshih <pkshih@realtek.com>; Yan-Hsuan Chuang <tony0620emma@gmail.com>; Kalle Valo
> <kvalo@codeaurora.org>
> Cc: linux-wireless@vger.kernel.org; netdev@vger.kernel.org; linux-kernel@vger.kernel.org;
> linux@endlessos.org; Jian-Hong Pan <jhp@endlessos.org>
> 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 <jhp@endlessos.org>

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?
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?

Thank you
--
Ping-Ke

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

[Kai-Heng Feng]

On Fri, Dec 10, 2021 at 5:00 PM Pkshih <pkshih@realtek.com> wrote:
>
> +Kai-Heng
>
> > -----Original Message-----
> > From: Jian-Hong Pan <jhp@endlessos.org>
> > Sent: Friday, December 10, 2021 4:17 PM
> > To: Pkshih <pkshih@realtek.com>; Yan-Hsuan Chuang <tony0620emma@gmail.com>; Kalle Valo
> > <kvalo@codeaurora.org>
> > Cc: linux-wireless@vger.kernel.org; netdev@vger.kernel.org; linux-kernel@vger.kernel.org;
> > linux@endlessos.org; Jian-Hong Pan <jhp@endlessos.org>
> > 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 <jhp@endlessos.org>
>
> 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.
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?

> 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.

Kai-Heng

>
> Thank you
> --
> Ping-Ke
>

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

[Jian-Hong Pan]

Kai-Heng Feng <kai.heng.feng@canonical.com> 於 2021年12月10日 週五 下午5:24寫道：
>
> On Fri, Dec 10, 2021 at 5:00 PM Pkshih <pkshih@realtek.com> wrote:
> >
> > +Kai-Heng
> >
> > > -----Original Message-----
> > > From: Jian-Hong Pan <jhp@endlessos.org>
> > > Sent: Friday, December 10, 2021 4:17 PM
> > > To: Pkshih <pkshih@realtek.com>; Yan-Hsuan Chuang <tony0620emma@gmail.com>; Kalle Valo
> > > <kvalo@codeaurora.org>
> > > Cc: linux-wireless@vger.kernel.org; netdev@vger.kernel.org; linux-kernel@vger.kernel.org;
> > > linux@endlessos.org; Jian-Hong Pan <jhp@endlessos.org>
> > > 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 <jhp@endlessos.org>
> >
> > 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!

> 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 Pan

> > 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.
>
> Kai-Heng
>
> >
> > Thank you
> > --
> > Ping-Ke
> >

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

[Pkshih]

> -----Original Message-----
> From: Jian-Hong Pan <jhp@endlessos.org>
> Sent: Friday, December 10, 2021 5:34 PM
> To: Kai-Heng Feng <kai.heng.feng@canonical.com>
> Cc: Pkshih <pkshih@realtek.com>; Yan-Hsuan Chuang <tony0620emma@gmail.com>; Kalle Valo
> <kvalo@codeaurora.org>; 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 <kai.heng.feng@canonical.com> 於 2021年12月10日 週五 下午5:24寫道：
> >
> > On Fri, Dec 10, 2021 at 5:00 PM Pkshih <pkshih@realtek.com> wrote:
> > >
> > > +Kai-Heng
> > >
> > > > -----Original Message-----
> > > > From: Jian-Hong Pan <jhp@endlessos.org>
> > > > Sent: Friday, December 10, 2021 4:17 PM
> > > > To: Pkshih <pkshih@realtek.com>; Yan-Hsuan Chuang <tony0620emma@gmail.com>; Kalle Valo
> > > > <kvalo@codeaurora.org>
> > > > Cc: linux-wireless@vger.kernel.org; netdev@vger.kernel.org; linux-kernel@vger.kernel.org;
> > > > linux@endlessos.org; Jian-Hong Pan <jhp@endlessos.org>
> > > > 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 <jhp@endlessos.org>
> > >
> > > 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.

> 
> > 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.

> 
> > > 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

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

[Jian-Hong Pan]

Pkshih <pkshih@realtek.com> 於 2021年12月11日 週六 下午2:31寫道：
>
>
> > -----Original Message-----
> > From: Jian-Hong Pan <jhp@endlessos.org>
> > Sent: Friday, December 10, 2021 5:34 PM
> > To: Kai-Heng Feng <kai.heng.feng@canonical.com>
> > Cc: Pkshih <pkshih@realtek.com>; Yan-Hsuan Chuang <tony0620emma@gmail.com>; Kalle Valo
> > <kvalo@codeaurora.org>; 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 <kai.heng.feng@canonical.com> 於 2021年12月10日 週五 下午5:24寫道：
> > >
> > > On Fri, Dec 10, 2021 at 5:00 PM Pkshih <pkshih@realtek.com> wrote:
> > > >
> > > > +Kai-Heng
> > > >
> > > > > -----Original Message-----
> > > > > From: Jian-Hong Pan <jhp@endlessos.org>
> > > > > Sent: Friday, December 10, 2021 4:17 PM
> > > > > To: Pkshih <pkshih@realtek.com>; Yan-Hsuan Chuang <tony0620emma@gmail.com>; Kalle Valo
> > > > > <kvalo@codeaurora.org>
> > > > > Cc: linux-wireless@vger.kernel.org; netdev@vger.kernel.org; linux-kernel@vger.kernel.org;
> > > > > linux@endlessos.org; Jian-Hong Pan <jhp@endlessos.org>
> > > > > 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 <jhp@endlessos.org>
> > > >
> > > > 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.
>
> >
> > > 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;
 }

> >
> > > > 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
>

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

[Pkshih]

> -----Original Message-----
> From: Jian-Hong Pan <jhp@endlessos.org>
> Sent: Monday, December 13, 2021 3:31 PM
> To: Pkshih <pkshih@realtek.com>
> Cc: Kai-Heng Feng <kai.heng.feng@canonical.com>; Yan-Hsuan Chuang <tony0620emma@gmail.com>; Kalle Valo
> <kvalo@codeaurora.org>; 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
> 
> Pkshih <pkshih@realtek.com> 於 2021年12月11日 週六 下午2:31寫道：
> >
> >
> > > -----Original Message-----
> > > From: Jian-Hong Pan <jhp@endlessos.org>
> > > Sent: Friday, December 10, 2021 5:34 PM
> > > To: Kai-Heng Feng <kai.heng.feng@canonical.com>
> > > Cc: Pkshih <pkshih@realtek.com>; Yan-Hsuan Chuang <tony0620emma@gmail.com>; Kalle Valo
> > > <kvalo@codeaurora.org>; 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 <kai.heng.feng@canonical.com> 於 2021年12月10日 週五 下午5:24寫道：
> > >
> > > > 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.

I meant that move "ref_cnt" into rtw_pci_link_ps() by [2], but you remove
the "ref_cnt". This leads lower performance, because it must turn off
ASPM after napi_poll() when we have high traffic.

In fact, Kai-Heng's patch is to leave ASPM before napi_poll(), and
"restore" ASPM setting. So, we still need "ref_cnt".


> 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;
>  }
> 

How about doing this thing only if 8821CE and Intel platform?
Could you help to add this?

--
Ping-ke

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

[Kai-Heng Feng]

On Mon, Dec 13, 2021 at 5:00 PM Pkshih <pkshih@realtek.com> wrote:
>
>
> > -----Original Message-----
> > From: Jian-Hong Pan <jhp@endlessos.org>
> > Sent: Monday, December 13, 2021 3:31 PM
> > To: Pkshih <pkshih@realtek.com>
> > Cc: Kai-Heng Feng <kai.heng.feng@canonical.com>; Yan-Hsuan Chuang <tony0620emma@gmail.com>; Kalle Valo
> > <kvalo@codeaurora.org>; 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
> >
> > Pkshih <pkshih@realtek.com> 於 2021年12月11日 週六 下午2:31寫道：
> > >
> > >
> > > > -----Original Message-----
> > > > From: Jian-Hong Pan <jhp@endlessos.org>
> > > > Sent: Friday, December 10, 2021 5:34 PM
> > > > To: Kai-Heng Feng <kai.heng.feng@canonical.com>
> > > > Cc: Pkshih <pkshih@realtek.com>; Yan-Hsuan Chuang <tony0620emma@gmail.com>; Kalle Valo
> > > > <kvalo@codeaurora.org>; 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 <kai.heng.feng@canonical.com> 於 2021年12月10日 週五 下午5:24寫道：
> > > >
> > > > > 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.
>
> I meant that move "ref_cnt" into rtw_pci_link_ps() by [2], but you remove
> the "ref_cnt". This leads lower performance, because it must turn off
> ASPM after napi_poll() when we have high traffic.
>
> In fact, Kai-Heng's patch is to leave ASPM before napi_poll(), and
> "restore" ASPM setting. So, we still need "ref_cnt".

I am working on the patch for proper upstream inclusion. Will send out soon.

Kai-Heng

>
>
> > 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;
> >  }
> >
>
> How about doing this thing only if 8821CE and Intel platform?
> Could you help to add this?
>
> --
> Ping-ke
>
>