Received-SPF: pass (google.com: domain of linux-wireless+bounces-8500-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Precedence: bulk
MIME-Version: 1.0
References: <20240523112131.31437-1-mingyen.hsieh@mediatek.com>
In-Reply-To: <20240523112131.31437-1-mingyen.hsieh@mediatek.com>
From: Sean Wang <sean.wang@kernel.org>
Date: Tue, 4 Jun 2024 16:11:58 -0700
Message-ID: <CAGp9LzrPUUjo8xLNdUddu=GCd2T8PP=Cw_+MKQCw23Emeg=n6Q@mail.gmail.com>
Subject: Re: [PATCH v2] wifi: mt76: mt792x: fix scheduler interference in drv
 own process
To: Mingyen Hsieh <mingyen.hsieh@mediatek.com>
Cc: nbd@nbd.name, lorenzo@kernel.org, deren.wu@mediatek.com, 
	Sean.Wang@mediatek.com, Soul.Huang@mediatek.com, Leon.Yen@mediatek.com, 
	Eric-SY.Chang@mediatek.com, km.lin@mediatek.com, robin.chiu@mediatek.com, 
	ch.yeh@mediatek.com, posh.sun@mediatek.com, Quan.Zhou@mediatek.com, 
	Ryder.Lee@mediatek.com, Shayne.Chen@mediatek.com, 
	linux-wireless@vger.kernel.org, linux-mediatek@lists.infradead.org, 
	Michael Lo <michael.lo@mediatek.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Hi,

On Thu, May 23, 2024 at 4:21=E2=80=AFAM Mingyen Hsieh
<mingyen.hsieh@mediatek.com> wrote:
>
> From: Michael Lo <michael.lo@mediatek.com>
>
> Add some time to wait for LP engine to complete its operation
> before polling pmctrl register.
>
> Signed-off-by: Michael Lo <michael.lo@mediatek.com>
> Signed-off-by: Ming Yen Hsieh <mingyen.hsieh@mediatek.com>

The new patch is an improvement over v1 as it avoids unnecessary
waiting in specific cases, such as when users are forced to disable
ASPM or when the host doesn't support ASPM as I suggested in v1. Thus,

Acked-by: Sean Wang <sean.wang@mediatek.com>

> ---
> v2: change funcion and variable naming.
> ---
>
>  drivers/net/wireless/mediatek/mt76/mt76.h     |  1 +
>  .../net/wireless/mediatek/mt76/mt7921/pci.c   |  3 +++
>  .../net/wireless/mediatek/mt76/mt7925/pci.c   |  3 +++
>  drivers/net/wireless/mediatek/mt76/mt792x.h   |  1 +
>  .../net/wireless/mediatek/mt76/mt792x_core.c  |  4 ++++
>  drivers/net/wireless/mediatek/mt76/pci.c      | 23 +++++++++++++++++++
>  6 files changed, 35 insertions(+)
>
> diff --git a/drivers/net/wireless/mediatek/mt76/mt76.h b/drivers/net/wire=
less/mediatek/mt76/mt76.h
> index 11b9f22ca7f3..4168104fa141 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt76.h
> +++ b/drivers/net/wireless/mediatek/mt76/mt76.h
> @@ -1081,6 +1081,7 @@ bool ____mt76_poll_msec(struct mt76_dev *dev, u32 o=
ffset, u32 mask, u32 val,
>
>  void mt76_mmio_init(struct mt76_dev *dev, void __iomem *regs);
>  void mt76_pci_disable_aspm(struct pci_dev *pdev);
> +bool mt76_pci_aspm_supported(struct pci_dev *pdev);
>
>  static inline u16 mt76_chip(struct mt76_dev *dev)
>  {
> diff --git a/drivers/net/wireless/mediatek/mt76/mt7921/pci.c b/drivers/ne=
t/wireless/mediatek/mt76/mt7921/pci.c
> index f768e9389ac6..ca54ae7deb54 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt7921/pci.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt7921/pci.c
> @@ -339,6 +339,9 @@ static int mt7921_pci_probe(struct pci_dev *pdev,
>         bus_ops->rmw =3D mt7921_rmw;
>         dev->mt76.bus =3D bus_ops;
>
> +       if (!mt7921_disable_aspm && mt76_pci_aspm_supported(pdev))
> +               dev->aspm_supported =3D true;
> +
>         ret =3D mt792xe_mcu_fw_pmctrl(dev);
>         if (ret)
>                 goto err_free_dev;
> diff --git a/drivers/net/wireless/mediatek/mt76/mt7925/pci.c b/drivers/ne=
t/wireless/mediatek/mt76/mt7925/pci.c
> index 07b74d492ce1..2449b3e6c396 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt7925/pci.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt7925/pci.c
> @@ -373,6 +373,9 @@ static int mt7925_pci_probe(struct pci_dev *pdev,
>         bus_ops->rmw =3D mt7925_rmw;
>         dev->mt76.bus =3D bus_ops;
>
> +       if (!mt7925_disable_aspm && mt76_pci_aspm_supported(pdev))
> +               dev->aspm_supported =3D true;
> +
>         ret =3D __mt792x_mcu_fw_pmctrl(dev);
>         if (ret)
>                 goto err_free_dev;
> diff --git a/drivers/net/wireless/mediatek/mt76/mt792x.h b/drivers/net/wi=
reless/mediatek/mt76/mt792x.h
> index 20578497a405..c2c42e5d650a 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt792x.h
> +++ b/drivers/net/wireless/mediatek/mt76/mt792x.h
> @@ -190,6 +190,7 @@ struct mt792x_dev {
>         bool fw_assert:1;
>         bool has_eht:1;
>         bool regd_in_progress:1;
> +       bool aspm_supported:1;
>         wait_queue_head_t wait;
>
>         struct work_struct init_work;
> diff --git a/drivers/net/wireless/mediatek/mt76/mt792x_core.c b/drivers/n=
et/wireless/mediatek/mt76/mt792x_core.c
> index a405af8d9052..588b24edfdd9 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt792x_core.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt792x_core.c
> @@ -766,6 +766,10 @@ int __mt792xe_mcu_drv_pmctrl(struct mt792x_dev *dev)
>
>         for (i =3D 0; i < MT792x_DRV_OWN_RETRY_COUNT; i++) {
>                 mt76_wr(dev, MT_CONN_ON_LPCTL, PCIE_LPCR_HOST_CLR_OWN);
> +
> +               if (dev->aspm_supported)
> +                       usleep_range(2000, 3000);
> +
>                 if (mt76_poll_msec_tick(dev, MT_CONN_ON_LPCTL,
>                                         PCIE_LPCR_HOST_OWN_SYNC, 0, 50, 1=
))
>                         break;
> diff --git a/drivers/net/wireless/mediatek/mt76/pci.c b/drivers/net/wirel=
ess/mediatek/mt76/pci.c
> index 4c1c159fbb62..b5031ca7f73f 100644
> --- a/drivers/net/wireless/mediatek/mt76/pci.c
> +++ b/drivers/net/wireless/mediatek/mt76/pci.c
> @@ -45,3 +45,26 @@ void mt76_pci_disable_aspm(struct pci_dev *pdev)
>                                            aspm_conf);
>  }
>  EXPORT_SYMBOL_GPL(mt76_pci_disable_aspm);
> +
> +bool mt76_pci_aspm_supported(struct pci_dev *pdev)
> +{
> +       struct pci_dev *parent =3D pdev->bus->self;
> +       u16 aspm_conf, parent_aspm_conf =3D 0;
> +       bool result =3D true;
> +
> +       pcie_capability_read_word(pdev, PCI_EXP_LNKCTL, &aspm_conf);
> +       aspm_conf &=3D PCI_EXP_LNKCTL_ASPMC;
> +       if (parent) {
> +               pcie_capability_read_word(parent, PCI_EXP_LNKCTL,
> +                                         &parent_aspm_conf);
> +               parent_aspm_conf &=3D PCI_EXP_LNKCTL_ASPMC;
> +       }
> +
> +       if (!aspm_conf && (!parent || !parent_aspm_conf)) {
> +               /* aspm already disabled */
> +               result =3D false;
> +       }
> +
> +       return result;
> +}
> +EXPORT_SYMBOL_GPL(mt76_pci_aspm_supported);
> --
> 2.18.0
>
>