Message ID | 20230601162537.1163270-1-kai.heng.feng@canonical.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:994d:0:b0:3d9:f83d:47d9 with SMTP id k13csp466507vqr; Thu, 1 Jun 2023 09:42:44 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ7XdRzA3tqa6BiMFlpjTD4z1UsZKYw1vRybhmjd+OAoMHZbLrgnYKI4YfOUUk/CuufqNbR0 X-Received: by 2002:a05:6a00:1823:b0:64d:6a78:157e with SMTP id y35-20020a056a00182300b0064d6a78157emr12738786pfa.28.1685637764344; Thu, 01 Jun 2023 09:42:44 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1685637764; cv=none; d=google.com; s=arc-20160816; b=cBPx1avRhaz+w0SkNmE096TwayTr1RaGESsu9jvM6V0/aYQc0X69e9QUO5vO1jwE+/ W0h4uGIBcY2Alez+19+f5921d2UBjXR6RlWdrc05I2vcl9S65dKbKx2UmWTIJyDCKZMf BEMtUX1cmKzZix61nGWMwcLCeKbdriDHwJ+/nlahQ6/QR74jPNRvMGIjgstAMigwwLxl 5kg3IcW624aCMg52HI20tk8CB9moztTR60g12nwklCHP/sMW/tsAHRqB9Pd9H500tdK2 IRv45tDs7GGzdWY5NQFGuqpiErJwG5jctUnhFZdG/CH9MTBE5PL9p+zGWrktPVnDtVkQ 6+4Q== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :message-id:date:subject:cc:to:from:dkim-signature; bh=axcyh+az/BBUzfpbsUua0mK6zDrWRXBjvxehcafqRw0=; b=OcRPQyPpR0h+RGHg9oPOFF9sqGArMmKxGYL65CsK36baVT3+bz5YkLmKEgFWdkAQZn OGLuWNql6PPyvN+MrSqaThfWxLyeD25mmYajbdshRsLXwVZeL+I5vdAy3qEsW0WYFqZr euM6uI5evOj/LBgNWeLYXLKADROdnBqYG8RRB0/zWFsXlmXogI4mz66fBw7of6a781PQ gWliQjDQgTEnfDNAxtheZOuL77BYY73GEkuLuFK/N9QOmfaL3F0WzNTA57WOq4aRfuvi nFdmyvWsEKTVXXp6fDhF7ItR05mOJK0h3oNYED36dw6HxL62YqT5e4SXioC9RHAATCoJ SFVg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@canonical.com header.s=20210705 header.b=OFimkuWy; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=canonical.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id bv188-20020a632ec5000000b0051909d663d8si3101851pgb.656.2023.06.01.09.42.32; Thu, 01 Jun 2023 09:42:44 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@canonical.com header.s=20210705 header.b=OFimkuWy; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=canonical.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231501AbjFAQ0l (ORCPT <rfc822;limurcpp@gmail.com> + 99 others); Thu, 1 Jun 2023 12:26:41 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:47470 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229880AbjFAQ0j (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Thu, 1 Jun 2023 12:26:39 -0400 Received: from smtp-relay-canonical-0.canonical.com (smtp-relay-canonical-0.canonical.com [185.125.188.120]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id E86A5138; Thu, 1 Jun 2023 09:26:37 -0700 (PDT) Received: from localhost.localdomain (unknown [10.101.196.174]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by smtp-relay-canonical-0.canonical.com (Postfix) with ESMTPSA id 134EE412AF; Thu, 1 Jun 2023 16:26:29 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=canonical.com; s=20210705; t=1685636793; bh=axcyh+az/BBUzfpbsUua0mK6zDrWRXBjvxehcafqRw0=; h=From:To:Cc:Subject:Date:Message-Id:MIME-Version; b=OFimkuWyFYRWSftA8jYcAl0tzkUQMW8d9D2Di/kyZMDMGb/0yzUZm6XoA80b9RKjo V2CFYG7zcW6O4GODt8uS55uXsWBYTRWaoz4pGBIET/WzX/6F9xWE1GJy3xOjZUdqwU bPmUmBHaCFwCJX6P7liZg01oCEpaAH96jmojFEvdvWiQS+9QRMtFvB15pVsue1V5CH nslXHWysanmAVO94TD+UABba+vrZi/63UDytkFWfCpzkNczYDSYV8hXeBMvWCoTsR8 PDNTQxDEvP5Z3t3tFzrzVhAA9rOQOu9+1jqInDnvTV0c6tKdJ3TOJoGlWUkCivqoIO C9mF8P1+72JcA== From: Kai-Heng Feng <kai.heng.feng@canonical.com> To: jesse.brandeburg@intel.com, anthony.l.nguyen@intel.com Cc: linux-pm@vger.kernel.org, Kai-Heng Feng <kai.heng.feng@canonical.com>, "David S. Miller" <davem@davemloft.net>, Eric Dumazet <edumazet@google.com>, Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>, intel-wired-lan@lists.osuosl.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH] e1000e: Use PME poll to circumvent unreliable ACPI wake Date: Fri, 2 Jun 2023 00:25:37 +0800 Message-Id: <20230601162537.1163270-1-kai.heng.feng@canonical.com> X-Mailer: git-send-email 2.34.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-4.6 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED, SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1767519304178553642?= X-GMAIL-MSGID: =?utf-8?q?1767519304178553642?= |
Series |
e1000e: Use PME poll to circumvent unreliable ACPI wake
|
|
Commit Message
Kai-Heng Feng
June 1, 2023, 4:25 p.m. UTC
On some I219 devices, ethernet cable plugging detection only works once
from PCI D3 state. Subsequent cable plugging does set PME bit correctly,
but device still doesn't get woken up.
Since I219 connects to the root complex directly, it relies on platform
firmware (ACPI) to wake it up. In this case, the GPE from _PRW only
works for first cable plugging but fails to notify the driver for
subsequent plugging events.
The issue was originally found on CNP, but the same issue can be found
on ADL too. So workaround the issue by continuing use PME poll after
first ACPI wake. As PME poll is always used, the runtime suspend
restriction for CNP can also be removed.
Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
---
drivers/net/ethernet/intel/e1000e/netdev.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
Comments
On Fri, 2023-06-02 at 00:25 +0800, Kai-Heng Feng wrote: > On some I219 devices, ethernet cable plugging detection only works once > from PCI D3 state. Subsequent cable plugging does set PME bit correctly, > but device still doesn't get woken up. Do we have a root cause on why things don't get woken up? This seems like an issue where something isn't getting reset after the first wakeup and so future ones are blocked. > Since I219 connects to the root complex directly, it relies on platform > firmware (ACPI) to wake it up. In this case, the GPE from _PRW only > works for first cable plugging but fails to notify the driver for > subsequent plugging events. > > The issue was originally found on CNP, but the same issue can be found > on ADL too. So workaround the issue by continuing use PME poll after > first ACPI wake. As PME poll is always used, the runtime suspend > restriction for CNP can also be removed. > > Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com> > --- > drivers/net/ethernet/intel/e1000e/netdev.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c > index bd7ef59b1f2e..f0e48f2bc3a2 100644 > --- a/drivers/net/ethernet/intel/e1000e/netdev.c > +++ b/drivers/net/ethernet/intel/e1000e/netdev.c > @@ -7021,6 +7021,8 @@ static __maybe_unused int e1000e_pm_runtime_resume(struct device *dev) > struct e1000_adapter *adapter = netdev_priv(netdev); > int rc; > > + pdev->pme_poll = true; > + > rc = __e1000_resume(pdev); > if (rc) > return rc; Doesn't this enable this too broadly. I know there are a number of devices that run under the e1000e and I would imagine that we don't want them all running with "pme_poll = true" do we? It seems like at a minimum we should only be setting this for specific platofrms or devices instead of on all of them. Also this seems like something we should be setting on the suspend side since it seems to be clared in the wakeup calls. Lastly I am not sure the first one is necessarily succeding. You might want to check the status of pme_poll before you run your first test. From what I can tell it looks like the initial state is true in pci_pm_init. If so it might be getting cleared after the first wakeup which is what causes your issues. > @@ -7682,7 +7684,7 @@ static int e1000_probe(struct pci_dev *pdev, const struct pci_device_id *ent) > > dev_pm_set_driver_flags(&pdev->dev, DPM_FLAG_SMART_PREPARE); > > - if (pci_dev_run_wake(pdev) && hw->mac.type != e1000_pch_cnp) > + if (pci_dev_run_wake(pdev)) > pm_runtime_put_noidle(&pdev->dev); > > return 0; I assume this is the original workaround that was put in to address this issue. Perhaps you should add a Fixes tag to this to identify which workaround this patch is meant to be replacing.
On Fri, Jun 2, 2023 at 4:24 AM Alexander H Duyck <alexander.duyck@gmail.com> wrote: > > On Fri, 2023-06-02 at 00:25 +0800, Kai-Heng Feng wrote: > > On some I219 devices, ethernet cable plugging detection only works once > > from PCI D3 state. Subsequent cable plugging does set PME bit correctly, > > but device still doesn't get woken up. > > Do we have a root cause on why things don't get woken up? This seems > like an issue where something isn't getting reset after the first > wakeup and so future ones are blocked. No we don't know the root cause. I guess the D3 wake isn't really tested under Windows because I219 doesn't use runtime D3 on Windows. > > > Since I219 connects to the root complex directly, it relies on platform > > firmware (ACPI) to wake it up. In this case, the GPE from _PRW only > > works for first cable plugging but fails to notify the driver for > > subsequent plugging events. > > > > The issue was originally found on CNP, but the same issue can be found > > on ADL too. So workaround the issue by continuing use PME poll after > > first ACPI wake. As PME poll is always used, the runtime suspend > > restriction for CNP can also be removed. > > > > Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com> > > --- > > drivers/net/ethernet/intel/e1000e/netdev.c | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c > > index bd7ef59b1f2e..f0e48f2bc3a2 100644 > > --- a/drivers/net/ethernet/intel/e1000e/netdev.c > > +++ b/drivers/net/ethernet/intel/e1000e/netdev.c > > @@ -7021,6 +7021,8 @@ static __maybe_unused int e1000e_pm_runtime_resume(struct device *dev) > > struct e1000_adapter *adapter = netdev_priv(netdev); > > int rc; > > > > + pdev->pme_poll = true; > > + > > rc = __e1000_resume(pdev); > > if (rc) > > return rc; > > Doesn't this enable this too broadly. I know there are a number of > devices that run under the e1000e and I would imagine that we don't > want them all running with "pme_poll = true" do we? Whack a mole isn't scaling, either. The generation between CNP and ADL are probably affected too. > > It seems like at a minimum we should only be setting this for specific > platofrms or devices instead of on all of them. > > Also this seems like something we should be setting on the suspend side > since it seems to be clared in the wakeup calls. pme_poll gets cleared on wakeup, and once it's cleared the device will be removed from pci_pme_list. To prevent that, reset pme_poll to true immediately on runtime resume. > > Lastly I am not sure the first one is necessarily succeding. You might > want to check the status of pme_poll before you run your first test. > From what I can tell it looks like the initial state is true in > pci_pm_init. If so it might be getting cleared after the first wakeup > which is what causes your issues. That's by design. pme_poll gets cleared when the hardware is capable to signal wakeup via PME# or ACPI GPE. For detected hardwares, the pme_poll will never be cleared. So this becomes tricky for the issue, since the ACPI GPE works for just one time, but never again. > > > @@ -7682,7 +7684,7 @@ static int e1000_probe(struct pci_dev *pdev, const struct pci_device_id *ent) > > > > dev_pm_set_driver_flags(&pdev->dev, DPM_FLAG_SMART_PREPARE); > > > > - if (pci_dev_run_wake(pdev) && hw->mac.type != e1000_pch_cnp) > > + if (pci_dev_run_wake(pdev)) > > pm_runtime_put_noidle(&pdev->dev); > > > > return 0; > > I assume this is the original workaround that was put in to address > this issue. Perhaps you should add a Fixes tag to this to identify > which workaround this patch is meant to be replacing. Another possibility is to remove runtime power management completely. I wonder why Windows keep the device at D0 all the time? Can Linux align with Windows? Kai-Heng
[Cc: linux-pci@vger.kernel.org] Dear Kai, Thank you for your patch. Am 02.06.23 um 03:46 schrieb Kai-Heng Feng: > On Fri, Jun 2, 2023 at 4:24 AM Alexander H Duyck wrote: >> >> On Fri, 2023-06-02 at 00:25 +0800, Kai-Heng Feng wrote: >>> On some I219 devices, ethernet cable plugging detection only works once >>> from PCI D3 state. Subsequent cable plugging does set PME bit correctly, >>> but device still doesn't get woken up. Could you please add the list of all the devices with the firmware version, you know this problem exists on? Please also add the URLs of the bug reports at the end of the commit message. Is that problem logged somehow? Could a log message be added first? >> Do we have a root cause on why things don't get woken up? This seems >> like an issue where something isn't getting reset after the first >> wakeup and so future ones are blocked. > > No we don't know the root cause. > I guess the D3 wake isn't really tested under Windows because I219 > doesn't use runtime D3 on Windows. How do you know? Where you able to look at the Microsoft Windows driver source code? >>> Since I219 connects to the root complex directly, it relies on platform >>> firmware (ACPI) to wake it up. In this case, the GPE from _PRW only >>> works for first cable plugging but fails to notify the driver for >>> subsequent plugging events. >>> >>> The issue was originally found on CNP, but the same issue can be found >>> on ADL too. So workaround the issue by continuing use PME poll after The verb is spelled with a space: work around. >>> first ACPI wake. As PME poll is always used, the runtime suspend >>> restriction for CNP can also be removed. When was that restriction for CNP added? >>> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com> >>> --- >>> drivers/net/ethernet/intel/e1000e/netdev.c | 4 +++- >>> 1 file changed, 3 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c >>> index bd7ef59b1f2e..f0e48f2bc3a2 100644 >>> --- a/drivers/net/ethernet/intel/e1000e/netdev.c >>> +++ b/drivers/net/ethernet/intel/e1000e/netdev.c >>> @@ -7021,6 +7021,8 @@ static __maybe_unused int e1000e_pm_runtime_resume(struct device *dev) >>> struct e1000_adapter *adapter = netdev_priv(netdev); >>> int rc; >>> >>> + pdev->pme_poll = true; >>> + >>> rc = __e1000_resume(pdev); >>> if (rc) >>> return rc; >> >> Doesn't this enable this too broadly. I know there are a number of >> devices that run under the e1000e and I would imagine that we don't >> want them all running with "pme_poll = true" do we? > > Whack a mole isn't scaling, either. > The generation between CNP and ADL are probably affected too. > >> It seems like at a minimum we should only be setting this for specific >> platofrms or devices instead of on all of them. >> >> Also this seems like something we should be setting on the suspend side >> since it seems to be cleared in the wakeup calls. > > pme_poll gets cleared on wakeup, and once it's cleared the device will > be removed from pci_pme_list. > > To prevent that, reset pme_poll to true immediately on runtime resume. > >> Lastly I am not sure the first one is necessarily succeeding. You might >> want to check the status of pme_poll before you run your first test. >> From what I can tell it looks like the initial state is true in >> pci_pm_init. If so it might be getting cleared after the first wakeup >> which is what causes your issues. > > That's by design. pme_poll gets cleared when the hardware is capable > to signal wakeup via PME# or ACPI GPE. For detected hardwares, the > pme_poll will never be cleared. > So this becomes tricky for the issue, since the ACPI GPE works for > just one time, but never again. > >>> @@ -7682,7 +7684,7 @@ static int e1000_probe(struct pci_dev *pdev, const struct pci_device_id *ent) >>> >>> dev_pm_set_driver_flags(&pdev->dev, DPM_FLAG_SMART_PREPARE); >>> >>> - if (pci_dev_run_wake(pdev) && hw->mac.type != e1000_pch_cnp) >>> + if (pci_dev_run_wake(pdev)) >>> pm_runtime_put_noidle(&pdev->dev); >>> >>> return 0; >> >> I assume this is the original workaround that was put in to address >> this issue. Perhaps you should add a Fixes tag to this to identify >> which workaround this patch is meant to be replacing. > > Another possibility is to remove runtime power management completely. > I wonder why Windows keep the device at D0 all the time? Who knows how to contact Intel’s driver developers for Microsoft Windows? > Can Linux align with Windows? Before deciding this, the power usage in the different states should be measured. Kind regards, Paul
Hi Paul, On Fri, Jun 2, 2023 at 4:43 PM Paul Menzel <pmenzel@molgen.mpg.de> wrote: > > [Cc: linux-pci@vger.kernel.org] > > Dear Kai, > > > Thank you for your patch. > > Am 02.06.23 um 03:46 schrieb Kai-Heng Feng: > > On Fri, Jun 2, 2023 at 4:24 AM Alexander H Duyck wrote: > >> > >> On Fri, 2023-06-02 at 00:25 +0800, Kai-Heng Feng wrote: > >>> On some I219 devices, ethernet cable plugging detection only works once > >>> from PCI D3 state. Subsequent cable plugging does set PME bit correctly, > >>> but device still doesn't get woken up. > > Could you please add the list of all the devices with the firmware > version, you know this problem exists on? Please also add the URLs of > the bug reports at the end of the commit message. Firmware do you mean the firmware on I219 device, or BIOS? > > Is that problem logged somehow? Could a log message be added first? There's nothing gets logged. When this happens the ACPI GPE is dead silent. > > >> Do we have a root cause on why things don't get woken up? This seems > >> like an issue where something isn't getting reset after the first > >> wakeup and so future ones are blocked. > > > > No we don't know the root cause. > > I guess the D3 wake isn't really tested under Windows because I219 > > doesn't use runtime D3 on Windows. > > How do you know? Where you able to look at the Microsoft Windows driver > source code? Device Manager shows the current PCI state. > > >>> Since I219 connects to the root complex directly, it relies on platform > >>> firmware (ACPI) to wake it up. In this case, the GPE from _PRW only > >>> works for first cable plugging but fails to notify the driver for > >>> subsequent plugging events. > >>> > >>> The issue was originally found on CNP, but the same issue can be found > >>> on ADL too. So workaround the issue by continuing use PME poll after > > The verb is spelled with a space: work around. Sure, will change it. > > >>> first ACPI wake. As PME poll is always used, the runtime suspend > >>> restriction for CNP can also be removed. > > When was that restriction for CNP added? The restriction for CNP+ was introduced by commit 459d69c407f9 ("e1000e: Disable runtime PM on CNP+") and modified by 3335369bad99 ("e1000e: Remove the runtime suspend restriction on CNP+"). > > >>> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com> > >>> --- > >>> drivers/net/ethernet/intel/e1000e/netdev.c | 4 +++- > >>> 1 file changed, 3 insertions(+), 1 deletion(-) > >>> > >>> diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c > >>> index bd7ef59b1f2e..f0e48f2bc3a2 100644 > >>> --- a/drivers/net/ethernet/intel/e1000e/netdev.c > >>> +++ b/drivers/net/ethernet/intel/e1000e/netdev.c > >>> @@ -7021,6 +7021,8 @@ static __maybe_unused int e1000e_pm_runtime_resume(struct device *dev) > >>> struct e1000_adapter *adapter = netdev_priv(netdev); > >>> int rc; > >>> > >>> + pdev->pme_poll = true; > >>> + > >>> rc = __e1000_resume(pdev); > >>> if (rc) > >>> return rc; > >> > >> Doesn't this enable this too broadly. I know there are a number of > >> devices that run under the e1000e and I would imagine that we don't > >> want them all running with "pme_poll = true" do we? > > > > Whack a mole isn't scaling, either. > > The generation between CNP and ADL are probably affected too. > > > >> It seems like at a minimum we should only be setting this for specific > >> platofrms or devices instead of on all of them. > >> > >> Also this seems like something we should be setting on the suspend side > >> since it seems to be cleared in the wakeup calls. > > > > pme_poll gets cleared on wakeup, and once it's cleared the device will > > be removed from pci_pme_list. > > > > To prevent that, reset pme_poll to true immediately on runtime resume. > > > >> Lastly I am not sure the first one is necessarily succeeding. You might > >> want to check the status of pme_poll before you run your first test. > >> From what I can tell it looks like the initial state is true in > >> pci_pm_init. If so it might be getting cleared after the first wakeup > >> which is what causes your issues. > > > > That's by design. pme_poll gets cleared when the hardware is capable > > to signal wakeup via PME# or ACPI GPE. For detected hardwares, the > > pme_poll will never be cleared. > > So this becomes tricky for the issue, since the ACPI GPE works for > > just one time, but never again. > > > >>> @@ -7682,7 +7684,7 @@ static int e1000_probe(struct pci_dev *pdev, const struct pci_device_id *ent) > >>> > >>> dev_pm_set_driver_flags(&pdev->dev, DPM_FLAG_SMART_PREPARE); > >>> > >>> - if (pci_dev_run_wake(pdev) && hw->mac.type != e1000_pch_cnp) > >>> + if (pci_dev_run_wake(pdev)) > >>> pm_runtime_put_noidle(&pdev->dev); > >>> > >>> return 0; > >> > >> I assume this is the original workaround that was put in to address > >> this issue. Perhaps you should add a Fixes tag to this to identify > >> which workaround this patch is meant to be replacing. > > > > Another possibility is to remove runtime power management completely. > > I wonder why Windows keep the device at D0 all the time? > > Who knows how to contact Intel’s driver developers for Microsoft Windows? Probably this mailing list? > > > Can Linux align with Windows? > > Before deciding this, the power usage in the different states should be > measured. The power usage doesn't matter if the device can't function properly. Kai-Heng > > > Kind regards, > > Paul
On 6/1/2023 19:25, Kai-Heng Feng wrote: > On some I219 devices, ethernet cable plugging detection only works once > from PCI D3 state. Subsequent cable plugging does set PME bit correctly, > but device still doesn't get woken up. > > Since I219 connects to the root complex directly, it relies on platform > firmware (ACPI) to wake it up. In this case, the GPE from _PRW only > works for first cable plugging but fails to notify the driver for > subsequent plugging events. > > The issue was originally found on CNP, but the same issue can be found > on ADL too. So workaround the issue by continuing use PME poll after > first ACPI wake. As PME poll is always used, the runtime suspend > restriction for CNP can also be removed. > > Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com> > --- > drivers/net/ethernet/intel/e1000e/netdev.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) Tested-by: Naama Meir <naamax.meir@linux.intel.com>
diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c index bd7ef59b1f2e..f0e48f2bc3a2 100644 --- a/drivers/net/ethernet/intel/e1000e/netdev.c +++ b/drivers/net/ethernet/intel/e1000e/netdev.c @@ -7021,6 +7021,8 @@ static __maybe_unused int e1000e_pm_runtime_resume(struct device *dev) struct e1000_adapter *adapter = netdev_priv(netdev); int rc; + pdev->pme_poll = true; + rc = __e1000_resume(pdev); if (rc) return rc; @@ -7682,7 +7684,7 @@ static int e1000_probe(struct pci_dev *pdev, const struct pci_device_id *ent) dev_pm_set_driver_flags(&pdev->dev, DPM_FLAG_SMART_PREPARE); - if (pci_dev_run_wake(pdev) && hw->mac.type != e1000_pch_cnp) + if (pci_dev_run_wake(pdev)) pm_runtime_put_noidle(&pdev->dev); return 0;