On Mon, Dec 13, 2021 at 5:00 PM Pkshih <pkshih@xxxxxxxxxxx> wrote: > > > > -----Original Message----- > > From: Jian-Hong Pan <jhp@xxxxxxxxxxxxx> > > Sent: Monday, December 13, 2021 3:31 PM > > To: Pkshih <pkshih@xxxxxxxxxxx> > > Cc: Kai-Heng Feng <kai.heng.feng@xxxxxxxxxxxxx>; Yan-Hsuan Chuang <tony0620emma@xxxxxxxxx>; Kalle Valo > > <kvalo@xxxxxxxxxxxxxx>; linux-wireless@xxxxxxxxxxxxxxx; netdev@xxxxxxxxxxxxxxx; > > linux-kernel@xxxxxxxxxxxxxxx; linux@xxxxxxxxxxxxx > > Subject: Re: [PATCH] rtw88: 8821c: disable the ASPM of RTL8821CE > > > > Pkshih <pkshih@xxxxxxxxxxx> 於 2021年12月11日 週六 下午2:31寫道： > > > > > > > > > > -----Original Message----- > > > > From: Jian-Hong Pan <jhp@xxxxxxxxxxxxx> > > > > Sent: Friday, December 10, 2021 5:34 PM > > > > To: Kai-Heng Feng <kai.heng.feng@xxxxxxxxxxxxx> > > > > Cc: Pkshih <pkshih@xxxxxxxxxxx>; Yan-Hsuan Chuang <tony0620emma@xxxxxxxxx>; Kalle Valo > > > > <kvalo@xxxxxxxxxxxxxx>; linux-wireless@xxxxxxxxxxxxxxx; netdev@xxxxxxxxxxxxxxx; > > > > linux-kernel@xxxxxxxxxxxxxxx; linux@xxxxxxxxxxxxx > > > > Subject: Re: [PATCH] rtw88: 8821c: disable the ASPM of RTL8821CE > > > > > > > > Kai-Heng Feng <kai.heng.feng@xxxxxxxxxxxxx> 於 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 > >
