[PATCH v3 1/2] e1000e: Only disable ASPM on 82573L devices

[PATCH v3 1/2] e1000e: Only disable ASPM on 82573L devices

[Matthew Garrett]

The 82537 errata and comment in e1000e_disable_l1aspm both agree that
only 82537L devices are affected. Limit the L1 disable to them.

Signed-off-by: Matthew Garrett <mjg@redhat.com>
---
 drivers/net/e1000e/netdev.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/drivers/net/e1000e/netdev.c b/drivers/net/e1000e/netdev.c
index 57f149b..27eed81 100644
--- a/drivers/net/e1000e/netdev.c
+++ b/drivers/net/e1000e/netdev.c
@@ -4642,6 +4642,10 @@ static void e1000e_disable_l1aspm(struct pci_dev *pdev)
 	 * Unfortunately this feature saves about 1W power consumption when
 	 * active.
 	 */
+
+	if (pdev->device != E1000_DEV_ID_82573L)
+		return;
+
 	pos = pci_find_capability(pdev, PCI_CAP_ID_EXP);
 	pci_read_config_word(pdev, pos + PCI_EXP_LNKCTL, &val);
 	if (val & 0x2) {
-- 
1.6.6.1

[PATCH v3 1/2] e1000e: Only disable ASPM on 82573L devices

[Matthew Garrett]

The 82537 errata and comment in e1000e_disable_l1aspm both agree that
only 82537L devices are affected. Limit the L1 disable to them.

Signed-off-by: Matthew Garrett <mjg@redhat.com>
---
 drivers/net/e1000e/netdev.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/drivers/net/e1000e/netdev.c b/drivers/net/e1000e/netdev.c
index 57f149b..27eed81 100644
--- a/drivers/net/e1000e/netdev.c
+++ b/drivers/net/e1000e/netdev.c
@@ -4642,6 +4642,10 @@ static void e1000e_disable_l1aspm(struct pci_dev *pdev)
 	 * Unfortunately this feature saves about 1W power consumption when
 	 * active.
 	 */
+
+	if (pdev->device != E1000_DEV_ID_82573L)
+		return;
+
 	pos = pci_find_capability(pdev, PCI_CAP_ID_EXP);
 	pci_read_config_word(pdev, pos + PCI_EXP_LNKCTL, &val);
 	if (val & 0x2) {
-- 
1.6.6.1


------------------------------------------------------------------------------
SOLARIS 10 is the OS for Data Centers - provides features such as DTrace,
Predictive Self Healing and Award Winning ZFS. Get Solaris 10 NOW
http://p.sf.net/sfu/solaris-dev2dev
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel&#174; Ethernet, visit http://communities.intel.com/community/wired

[PATCH v3 2/2] e1000e: Don't disable jumbo frames on 82573L due to eeprom contents

[Matthew Garrett]

According to the 82573L errata, jumbo frames will work correctly if ASPM
is disabled. Since we disable ASPM on these devices, leave jumbo frames
enabled.

Signed-off-by: Matthew Garrett <mjg@redhat.com>
---
Updated to provide the right chip name 

 drivers/net/e1000e/82571.c |   11 ++---------
 1 files changed, 2 insertions(+), 9 deletions(-)

diff --git a/drivers/net/e1000e/82571.c b/drivers/net/e1000e/82571.c
index 02d67d0..f26edf7 100644
--- a/drivers/net/e1000e/82571.c
+++ b/drivers/net/e1000e/82571.c
@@ -330,7 +330,6 @@ static s32 e1000_get_variants_82571(struct e1000_adapter *adapter)
 	struct e1000_hw *hw = &adapter->hw;
 	static int global_quad_port_a; /* global port a indication */
 	struct pci_dev *pdev = adapter->pdev;
-	u16 eeprom_data = 0;
 	int is_port_b = er32(STATUS) & E1000_STATUS_FUNC_1;
 	s32 rc;
 
@@ -381,16 +380,10 @@ static s32 e1000_get_variants_82571(struct e1000_adapter *adapter)
 		if (pdev->device == E1000_DEV_ID_82571EB_SERDES_QUAD)
 			adapter->flags &= ~FLAG_HAS_WOL;
 		break;
-
 	case e1000_82573:
 		if (pdev->device == E1000_DEV_ID_82573L) {
-			if (e1000_read_nvm(&adapter->hw, NVM_INIT_3GIO_3, 1,
-				       &eeprom_data) < 0)
-				break;
-			if (!(eeprom_data & NVM_WORD1A_ASPM_MASK)) {
-				adapter->flags |= FLAG_HAS_JUMBO_FRAMES;
-				adapter->max_hw_frame_size = DEFAULT_JUMBO;
-			}
+			adapter->flags |= FLAG_HAS_JUMBO_FRAMES;
+			adapter->max_hw_frame_size = DEFAULT_JUMBO;
 		}
 		break;
 	default:
-- 
1.6.6.1

[PATCH v3 2/2] e1000e: Don't disable jumbo frames on 82573L due to eeprom contents

[Matthew Garrett]

According to the 82573L errata, jumbo frames will work correctly if ASPM
is disabled. Since we disable ASPM on these devices, leave jumbo frames
enabled.

Signed-off-by: Matthew Garrett <mjg@redhat.com>
---
Updated to provide the right chip name 

 drivers/net/e1000e/82571.c |   11 ++---------
 1 files changed, 2 insertions(+), 9 deletions(-)

diff --git a/drivers/net/e1000e/82571.c b/drivers/net/e1000e/82571.c
index 02d67d0..f26edf7 100644
--- a/drivers/net/e1000e/82571.c
+++ b/drivers/net/e1000e/82571.c
@@ -330,7 +330,6 @@ static s32 e1000_get_variants_82571(struct e1000_adapter *adapter)
 	struct e1000_hw *hw = &adapter->hw;
 	static int global_quad_port_a; /* global port a indication */
 	struct pci_dev *pdev = adapter->pdev;
-	u16 eeprom_data = 0;
 	int is_port_b = er32(STATUS) & E1000_STATUS_FUNC_1;
 	s32 rc;
 
@@ -381,16 +380,10 @@ static s32 e1000_get_variants_82571(struct e1000_adapter *adapter)
 		if (pdev->device == E1000_DEV_ID_82571EB_SERDES_QUAD)
 			adapter->flags &= ~FLAG_HAS_WOL;
 		break;
-
 	case e1000_82573:
 		if (pdev->device == E1000_DEV_ID_82573L) {
-			if (e1000_read_nvm(&adapter->hw, NVM_INIT_3GIO_3, 1,
-				       &eeprom_data) < 0)
-				break;
-			if (!(eeprom_data & NVM_WORD1A_ASPM_MASK)) {
-				adapter->flags |= FLAG_HAS_JUMBO_FRAMES;
-				adapter->max_hw_frame_size = DEFAULT_JUMBO;
-			}
+			adapter->flags |= FLAG_HAS_JUMBO_FRAMES;
+			adapter->max_hw_frame_size = DEFAULT_JUMBO;
 		}
 		break;
 	default:
-- 
1.6.6.1


------------------------------------------------------------------------------
SOLARIS 10 is the OS for Data Centers - provides features such as DTrace,
Predictive Self Healing and Award Winning ZFS. Get Solaris 10 NOW
http://p.sf.net/sfu/solaris-dev2dev
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel&#174; Ethernet, visit http://communities.intel.com/community/wired

RE: [E1000-devel] [PATCH v3 2/2] e1000e: Don't disable jumbo frames on 82573L due to eeprom contents

[Allan, Bruce W]

On Thursday, February 11, 2010 10:15 AM, Matthew Garrett wrote:
> According to the 82573L errata, jumbo frames will work correctly if
> ASPM 
> is disabled. Since we disable ASPM on these devices, leave jumbo
> frames 
> enabled.
> 
> Signed-off-by: Matthew Garrett <mjg@redhat.com>
> ---
> Updated to provide the right chip name
> 
>  drivers/net/e1000e/82571.c |   11 ++---------
>  1 files changed, 2 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/net/e1000e/82571.c b/drivers/net/e1000e/82571.c
> index 02d67d0..f26edf7 100644
> --- a/drivers/net/e1000e/82571.c
> +++ b/drivers/net/e1000e/82571.c
> @@ -330,7 +330,6 @@ static s32 e1000_get_variants_82571(struct
>  	e1000_adapter *adapter) struct e1000_hw *hw = &adapter->hw;
>  	static int global_quad_port_a; /* global port a indication */
>  	struct pci_dev *pdev = adapter->pdev;
> -	u16 eeprom_data = 0;
>  	int is_port_b = er32(STATUS) & E1000_STATUS_FUNC_1;
>  	s32 rc;
> 
> @@ -381,16 +380,10 @@ static s32 e1000_get_variants_82571(struct
>  		e1000_adapter *adapter) if (pdev->device ==
>  			E1000_DEV_ID_82571EB_SERDES_QUAD) adapter->flags &= ~FLAG_HAS_WOL;
>  		break;
> -
>  	case e1000_82573:
>  		if (pdev->device == E1000_DEV_ID_82573L) {
> -			if (e1000_read_nvm(&adapter->hw, NVM_INIT_3GIO_3, 1,
> -				       &eeprom_data) < 0)
> -				break;
> -			if (!(eeprom_data & NVM_WORD1A_ASPM_MASK)) {
> -				adapter->flags |= FLAG_HAS_JUMBO_FRAMES;
> -				adapter->max_hw_frame_size = DEFAULT_JUMBO;
> -			}
> +			adapter->flags |= FLAG_HAS_JUMBO_FRAMES;
> +			adapter->max_hw_frame_size = DEFAULT_JUMBO;
>  		}
>  		break;
>  	default:

Actually, the driver only disables L1 ASPM, it does not disable L0s ASPM.  The erratum is unclear whether jumbo frames are not supported in L0s, L1 or both modes (the code suggests both modes must be disabled).  I will look into this further.

Thanks,
Bruce.

Re: [PATCH v3 2/2] e1000e: Don't disable jumbo frames on 82573L due to eeprom contents

[Allan, Bruce W]

On Thursday, February 11, 2010 10:15 AM, Matthew Garrett wrote:
> According to the 82573L errata, jumbo frames will work correctly if
> ASPM 
> is disabled. Since we disable ASPM on these devices, leave jumbo
> frames 
> enabled.
> 
> Signed-off-by: Matthew Garrett <mjg@redhat.com>
> ---
> Updated to provide the right chip name
> 
>  drivers/net/e1000e/82571.c |   11 ++---------
>  1 files changed, 2 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/net/e1000e/82571.c b/drivers/net/e1000e/82571.c
> index 02d67d0..f26edf7 100644
> --- a/drivers/net/e1000e/82571.c
> +++ b/drivers/net/e1000e/82571.c
> @@ -330,7 +330,6 @@ static s32 e1000_get_variants_82571(struct
>  	e1000_adapter *adapter) struct e1000_hw *hw = &adapter->hw;
>  	static int global_quad_port_a; /* global port a indication */
>  	struct pci_dev *pdev = adapter->pdev;
> -	u16 eeprom_data = 0;
>  	int is_port_b = er32(STATUS) & E1000_STATUS_FUNC_1;
>  	s32 rc;
> 
> @@ -381,16 +380,10 @@ static s32 e1000_get_variants_82571(struct
>  		e1000_adapter *adapter) if (pdev->device ==
>  			E1000_DEV_ID_82571EB_SERDES_QUAD) adapter->flags &= ~FLAG_HAS_WOL;
>  		break;
> -
>  	case e1000_82573:
>  		if (pdev->device == E1000_DEV_ID_82573L) {
> -			if (e1000_read_nvm(&adapter->hw, NVM_INIT_3GIO_3, 1,
> -				       &eeprom_data) < 0)
> -				break;
> -			if (!(eeprom_data & NVM_WORD1A_ASPM_MASK)) {
> -				adapter->flags |= FLAG_HAS_JUMBO_FRAMES;
> -				adapter->max_hw_frame_size = DEFAULT_JUMBO;
> -			}
> +			adapter->flags |= FLAG_HAS_JUMBO_FRAMES;
> +			adapter->max_hw_frame_size = DEFAULT_JUMBO;
>  		}
>  		break;
>  	default:

Actually, the driver only disables L1 ASPM, it does not disable L0s ASPM.  The erratum is unclear whether jumbo frames are not supported in L0s, L1 or both modes (the code suggests both modes must be disabled).  I will look into this further.

Thanks,
Bruce.
------------------------------------------------------------------------------
Download Intel&#174; Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel&#174; Ethernet, visit http://communities.intel.com/community/wired

RE: [E1000-devel] [PATCH v3 1/2] e1000e: Only disable ASPM on 82573L devices

[Allan, Bruce W]

On Thursday, February 11, 2010 10:15 AM, Matthew Garrett wrote:
> The 82537 errata and comment in e1000e_disable_l1aspm both agree that
> only 82537L devices are affected. Limit the L1 disable to them.
> 
> Signed-off-by: Matthew Garrett <mjg@redhat.com>
> ---
>  drivers/net/e1000e/netdev.c |    4 ++++
>  1 files changed, 4 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/net/e1000e/netdev.c b/drivers/net/e1000e/netdev.c
> index 57f149b..27eed81 100644
> --- a/drivers/net/e1000e/netdev.c
> +++ b/drivers/net/e1000e/netdev.c
> @@ -4642,6 +4642,10 @@ static void e1000e_disable_l1aspm(struct
> pci_dev *pdev) 
>  	 * Unfortunately this feature saves about 1W power consumption when
>  	 * active.
>  	 */
> +
> +	if (pdev->device != E1000_DEV_ID_82573L)
> +		return;
> +
>  	pos = pci_find_capability(pdev, PCI_CAP_ID_EXP);
>  	pci_read_config_word(pdev, pos + PCI_EXP_LNKCTL, &val);
>  	if (val & 0x2) {

Hi Matthew,

Exactly which erratum are you referring to?  Erratum 17 in the 82573 Specification Update?  If that is the case, I see the possibility of another interpretation of the erratum which suggests the possibility of the same issue on other variants of the 82573 when using standard frame sizes and ASPM enabled.  Not to mention, I believe there may be other parts (82574 perhaps) that will have issues with L1 ASPM enabled.  I will follow-up with the folks who did the investigation that resulted in the erratum in order to get a clearer picture of all this, and take a look into other parts that may likewise be affected.

Thanks,
Bruce.

Re: [PATCH v3 1/2] e1000e: Only disable ASPM on 82573L devices

[Allan, Bruce W]

On Thursday, February 11, 2010 10:15 AM, Matthew Garrett wrote:
> The 82537 errata and comment in e1000e_disable_l1aspm both agree that
> only 82537L devices are affected. Limit the L1 disable to them.
> 
> Signed-off-by: Matthew Garrett <mjg@redhat.com>
> ---
>  drivers/net/e1000e/netdev.c |    4 ++++
>  1 files changed, 4 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/net/e1000e/netdev.c b/drivers/net/e1000e/netdev.c
> index 57f149b..27eed81 100644
> --- a/drivers/net/e1000e/netdev.c
> +++ b/drivers/net/e1000e/netdev.c
> @@ -4642,6 +4642,10 @@ static void e1000e_disable_l1aspm(struct
> pci_dev *pdev) 
>  	 * Unfortunately this feature saves about 1W power consumption when
>  	 * active.
>  	 */
> +
> +	if (pdev->device != E1000_DEV_ID_82573L)
> +		return;
> +
>  	pos = pci_find_capability(pdev, PCI_CAP_ID_EXP);
>  	pci_read_config_word(pdev, pos + PCI_EXP_LNKCTL, &val);
>  	if (val & 0x2) {

Hi Matthew,

Exactly which erratum are you referring to?  Erratum 17 in the 82573 Specification Update?  If that is the case, I see the possibility of another interpretation of the erratum which suggests the possibility of the same issue on other variants of the 82573 when using standard frame sizes and ASPM enabled.  Not to mention, I believe there may be other parts (82574 perhaps) that will have issues with L1 ASPM enabled.  I will follow-up with the folks who did the investigation that resulted in the erratum in order to get a clearer picture of all this, and take a look into other parts that may likewise be affected.

Thanks,
Bruce.
------------------------------------------------------------------------------
Download Intel&#174; Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel&#174; Ethernet, visit http://communities.intel.com/community/wired

Re: [E1000-devel] [PATCH v3 1/2] e1000e: Only disable ASPM on 82573L devices

[Matthew Garrett]

On Fri, Feb 19, 2010 at 01:53:01PM -0800, Allan, Bruce W wrote:

> Exactly which erratum are you referring to?  Erratum 17 in the 82573 
> Specification Update?  If that is the case, I see the possibility of 
> another interpretation of the erratum which suggests the possibility 
> of the same issue on other variants of the 82573 when using standard 
> frame sizes and ASPM enabled.  Not to mention, I believe there may be 
> other parts (82574 perhaps) that will have issues with L1 ASPM 
> enabled.  I will follow-up with the folks who did the investigation 
> that resulted in the erratum in order to get a clearer picture of all 
> this, and take a look into other parts that may likewise be affected.

Ah, yes - I see that it could be interpreted that way. The description 
seems to suggest that it's only relevant if ERT is enabled, which is 
required for jumbo frames. I'm not entirely clear on whether ERT is 
enabled in other circumstances? If not, we ought to be able to limit 
this to the L device - if not, it should be done on E and V as well.

The 82574 specification update doesn't mention any ASPM errata, but if 
you're able to check then that would be great. My main aim here is to 
try to get it turned back on on hardware where this works, since it's a 
measurable power saving.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH v3 1/2] e1000e: Only disable ASPM on 82573L devices

[Matthew Garrett]

On Fri, Feb 19, 2010 at 01:53:01PM -0800, Allan, Bruce W wrote:

> Exactly which erratum are you referring to?  Erratum 17 in the 82573 
> Specification Update?  If that is the case, I see the possibility of 
> another interpretation of the erratum which suggests the possibility 
> of the same issue on other variants of the 82573 when using standard 
> frame sizes and ASPM enabled.  Not to mention, I believe there may be 
> other parts (82574 perhaps) that will have issues with L1 ASPM 
> enabled.  I will follow-up with the folks who did the investigation 
> that resulted in the erratum in order to get a clearer picture of all 
> this, and take a look into other parts that may likewise be affected.

Ah, yes - I see that it could be interpreted that way. The description 
seems to suggest that it's only relevant if ERT is enabled, which is 
required for jumbo frames. I'm not entirely clear on whether ERT is 
enabled in other circumstances? If not, we ought to be able to limit 
this to the L device - if not, it should be done on E and V as well.

The 82574 specification update doesn't mention any ASPM errata, but if 
you're able to check then that would be great. My main aim here is to 
try to get it turned back on on hardware where this works, since it's a 
measurable power saving.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

------------------------------------------------------------------------------
Download Intel® Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel® Ethernet, visit http://communities.intel.com/community/wired