| Message ID | 20211210081659.4621-1-jhp@endlessos.org |
|---|---|
| State | New |
| Headers | show |
| Series | rtw88: 8821c: disable the ASPM of RTL8821CE | expand |
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 >
> -----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
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 > >
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);
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(-)