[Intel-wired-lan] [PATCH iwl-net v1 1/1] e1000e: Fix S0ix residency on Meteor Lake corporate systems

[Intel-wired-lan] [PATCH iwl-net v1 1/1] e1000e: Fix S0ix residency on Meteor Lake corporate systems

[Vitaly Lifshits]

From: Dima Ruinskiy <dima.ruinskiy@intel.com>

On vPro systems,the  configuration of the I219-LM to achieve power
gating and S0ix residency is split between the driver and the CSME FW.
It was discovered that in some scenarios, where the network cable is
connected and then disconnected, S0ix residency is not always reached.
This was root-caused to a subset of I219-LM register writes that are not
performed by the CSME FW. Therefore, the driver should perform these
register writes on corporate setups, regardless of the CSME FW state.

Fixes: cc23f4f0b6b9 ("e1000e: Add support for Meteor Lake")
Signed-off-by: Dima Ruinskiy <dima.ruinskiy@intel.com>
Signed-off-by: Vitaly.Lifshits <vitaly.lifshits@intel.com>
Signed-off-by: Vitaly Lifshits <vitaly.lifshits@intel.com>
---
 drivers/net/ethernet/intel/e1000e/netdev.c | 132 ++++++++++-----------
 1 file changed, 66 insertions(+), 66 deletions(-)

diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
index da5c59daf8ba..3cd161c6672b 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -6363,49 +6363,49 @@ static void e1000e_s0ix_entry_flow(struct e1000_adapter *adapter)
 		mac_data |= E1000_EXTCNF_CTRL_GATE_PHY_CFG;
 		ew32(EXTCNF_CTRL, mac_data);
 
-		/* Enable the Dynamic Power Gating in the MAC */
-		mac_data = er32(FEXTNVM7);
-		mac_data |= BIT(22);
-		ew32(FEXTNVM7, mac_data);
-
 		/* Disable disconnected cable conditioning for Power Gating */
 		mac_data = er32(DPGFR);
 		mac_data |= BIT(2);
 		ew32(DPGFR, mac_data);
 
-		/* Don't wake from dynamic Power Gating with clock request */
-		mac_data = er32(FEXTNVM12);
-		mac_data |= BIT(12);
-		ew32(FEXTNVM12, mac_data);
-
-		/* Ungate PGCB clock */
-		mac_data = er32(FEXTNVM9);
-		mac_data &= ~BIT(28);
-		ew32(FEXTNVM9, mac_data);
-
-		/* Enable K1 off to enable mPHY Power Gating */
-		mac_data = er32(FEXTNVM6);
-		mac_data |= BIT(31);
-		ew32(FEXTNVM6, mac_data);
-
-		/* Enable mPHY power gating for any link and speed */
-		mac_data = er32(FEXTNVM8);
-		mac_data |= BIT(9);
-		ew32(FEXTNVM8, mac_data);
-
 		/* Enable the Dynamic Clock Gating in the DMA and MAC */
 		mac_data = er32(CTRL_EXT);
 		mac_data |= E1000_CTRL_EXT_DMA_DYN_CLK_EN;
 		ew32(CTRL_EXT, mac_data);
-
-		/* No MAC DPG gating SLP_S0 in modern standby
-		 * Switch the logic of the lanphypc to use PMC counter
-		 */
-		mac_data = er32(FEXTNVM5);
-		mac_data |= BIT(7);
-		ew32(FEXTNVM5, mac_data);
 	}
 
+	/* Enable the Dynamic Power Gating in the MAC */
+	mac_data = er32(FEXTNVM7);
+	mac_data |= BIT(22);
+	ew32(FEXTNVM7, mac_data);
+
+	/* Don't wake from dynamic Power Gating with clock request */
+	mac_data = er32(FEXTNVM12);
+	mac_data |= BIT(12);
+	ew32(FEXTNVM12, mac_data);
+
+	/* Ungate PGCB clock */
+	mac_data = er32(FEXTNVM9);
+	mac_data &= ~BIT(28);
+	ew32(FEXTNVM9, mac_data);
+
+	/* Enable K1 off to enable mPHY Power Gating */
+	mac_data = er32(FEXTNVM6);
+	mac_data |= BIT(31);
+	ew32(FEXTNVM6, mac_data);
+
+	/* Enable mPHY power gating for any link and speed */
+	mac_data = er32(FEXTNVM8);
+	mac_data |= BIT(9);
+	ew32(FEXTNVM8, mac_data);
+
+	/* No MAC DPG gating SLP_S0 in modern standby
+	 * Switch the logic of the lanphypc to use PMC counter
+	 */
+	mac_data = er32(FEXTNVM5);
+	mac_data |= BIT(7);
+	ew32(FEXTNVM5, mac_data);
+
 	/* Disable the time synchronization clock */
 	mac_data = er32(FEXTNVM7);
 	mac_data |= BIT(31);
@@ -6498,33 +6498,6 @@ static void e1000e_s0ix_exit_flow(struct e1000_adapter *adapter)
 	} else {
 		/* Request driver unconfigure the device from S0ix */
 
-		/* Disable the Dynamic Power Gating in the MAC */
-		mac_data = er32(FEXTNVM7);
-		mac_data &= 0xFFBFFFFF;
-		ew32(FEXTNVM7, mac_data);
-
-		/* Disable mPHY power gating for any link and speed */
-		mac_data = er32(FEXTNVM8);
-		mac_data &= ~BIT(9);
-		ew32(FEXTNVM8, mac_data);
-
-		/* Disable K1 off */
-		mac_data = er32(FEXTNVM6);
-		mac_data &= ~BIT(31);
-		ew32(FEXTNVM6, mac_data);
-
-		/* Disable Ungate PGCB clock */
-		mac_data = er32(FEXTNVM9);
-		mac_data |= BIT(28);
-		ew32(FEXTNVM9, mac_data);
-
-		/* Cancel not waking from dynamic
-		 * Power Gating with clock request
-		 */
-		mac_data = er32(FEXTNVM12);
-		mac_data &= ~BIT(12);
-		ew32(FEXTNVM12, mac_data);
-
 		/* Cancel disable disconnected cable conditioning
 		 * for Power Gating
 		 */
@@ -6537,13 +6510,6 @@ static void e1000e_s0ix_exit_flow(struct e1000_adapter *adapter)
 		mac_data &= 0xFFF7FFFF;
 		ew32(CTRL_EXT, mac_data);
 
-		/* Revert the lanphypc logic to use the internal Gbe counter
-		 * and not the PMC counter
-		 */
-		mac_data = er32(FEXTNVM5);
-		mac_data &= 0xFFFFFF7F;
-		ew32(FEXTNVM5, mac_data);
-
 		/* Enable the periodic inband message,
 		 * Request PCIe clock in K1 page770_17[10:9] =01b
 		 */
@@ -6581,6 +6547,40 @@ static void e1000e_s0ix_exit_flow(struct e1000_adapter *adapter)
 	mac_data &= ~BIT(31);
 	mac_data |= BIT(0);
 	ew32(FEXTNVM7, mac_data);
+
+	/* Disable the Dynamic Power Gating in the MAC */
+	mac_data = er32(FEXTNVM7);
+	mac_data &= 0xFFBFFFFF;
+	ew32(FEXTNVM7, mac_data);
+
+	/* Disable mPHY power gating for any link and speed */
+	mac_data = er32(FEXTNVM8);
+	mac_data &= ~BIT(9);
+	ew32(FEXTNVM8, mac_data);
+
+	/* Disable K1 off */
+	mac_data = er32(FEXTNVM6);
+	mac_data &= ~BIT(31);
+	ew32(FEXTNVM6, mac_data);
+
+	/* Disable Ungate PGCB clock */
+	mac_data = er32(FEXTNVM9);
+	mac_data |= BIT(28);
+	ew32(FEXTNVM9, mac_data);
+
+	/* Cancel not waking from dynamic
+	 * Power Gating with clock request
+	 */
+	mac_data = er32(FEXTNVM12);
+	mac_data &= ~BIT(12);
+	ew32(FEXTNVM12, mac_data);
+
+	/* Revert the lanphypc logic to use the internal Gbe counter
+	 * and not the PMC counter
+	 */
+	mac_data = er32(FEXTNVM5);
+	mac_data &= 0xFFFFFF7F;
+	ew32(FEXTNVM5, mac_data);
 }
 
 static int e1000e_pm_freeze(struct device *dev)
-- 
2.34.1

Re: [Intel-wired-lan] [PATCH iwl-net v1 1/1] e1000e: Fix S0ix residency on Meteor Lake corporate systems

[Paul Menzel]

Dear Vitaly,


Thank you for the patch.

Am 28.05.24 um 12:33 schrieb Vitaly Lifshits:
> From: Dima Ruinskiy <dima.ruinskiy@intel.com>
> 
> On vPro systems,the  configuration of the I219-LM to achieve power

s/,the  /, the /

> gating and S0ix residency is split between the driver and the CSME FW.
> It was discovered that in some scenarios, where the network cable is
> connected and then disconnected, S0ix residency is not always reached.

Disconnected at any point, or just during suspend?

Any URL to the reports?

> This was root-caused to a subset of I219-LM register writes that are not
> performed by the CSME FW. Therefore, the driver should perform these
> register writes on corporate setups, regardless of the CSME FW state.

Is that documented somewhere?

Please add more information about the affected systems, and the test 
environment (firmware versions, …).

> Fixes: cc23f4f0b6b9 ("e1000e: Add support for Meteor Lake")
> Signed-off-by: Dima Ruinskiy <dima.ruinskiy@intel.com>
> Signed-off-by: Vitaly.Lifshits <vitaly.lifshits@intel.com>

The line above with the dot can be removed.

> Signed-off-by: Vitaly Lifshits <vitaly.lifshits@intel.com>
> ---
>   drivers/net/ethernet/intel/e1000e/netdev.c | 132 ++++++++++-----------
>   1 file changed, 66 insertions(+), 66 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
> index da5c59daf8ba..3cd161c6672b 100644
> --- a/drivers/net/ethernet/intel/e1000e/netdev.c
> +++ b/drivers/net/ethernet/intel/e1000e/netdev.c
> @@ -6363,49 +6363,49 @@ static void e1000e_s0ix_entry_flow(struct e1000_adapter *adapter)
>   		mac_data |= E1000_EXTCNF_CTRL_GATE_PHY_CFG;
>   		ew32(EXTCNF_CTRL, mac_data);
>   
> -		/* Enable the Dynamic Power Gating in the MAC */
> -		mac_data = er32(FEXTNVM7);
> -		mac_data |= BIT(22);
> -		ew32(FEXTNVM7, mac_data);
> -
>   		/* Disable disconnected cable conditioning for Power Gating */
>   		mac_data = er32(DPGFR);
>   		mac_data |= BIT(2);
>   		ew32(DPGFR, mac_data);
>   
> -		/* Don't wake from dynamic Power Gating with clock request */
> -		mac_data = er32(FEXTNVM12);
> -		mac_data |= BIT(12);
> -		ew32(FEXTNVM12, mac_data);
> -
> -		/* Ungate PGCB clock */
> -		mac_data = er32(FEXTNVM9);
> -		mac_data &= ~BIT(28);
> -		ew32(FEXTNVM9, mac_data);
> -
> -		/* Enable K1 off to enable mPHY Power Gating */
> -		mac_data = er32(FEXTNVM6);
> -		mac_data |= BIT(31);
> -		ew32(FEXTNVM6, mac_data);
> -
> -		/* Enable mPHY power gating for any link and speed */
> -		mac_data = er32(FEXTNVM8);
> -		mac_data |= BIT(9);
> -		ew32(FEXTNVM8, mac_data);
> -
>   		/* Enable the Dynamic Clock Gating in the DMA and MAC */
>   		mac_data = er32(CTRL_EXT);
>   		mac_data |= E1000_CTRL_EXT_DMA_DYN_CLK_EN;
>   		ew32(CTRL_EXT, mac_data);
> -
> -		/* No MAC DPG gating SLP_S0 in modern standby
> -		 * Switch the logic of the lanphypc to use PMC counter
> -		 */
> -		mac_data = er32(FEXTNVM5);
> -		mac_data |= BIT(7);
> -		ew32(FEXTNVM5, mac_data);
>   	}
>   
> +	/* Enable the Dynamic Power Gating in the MAC */
> +	mac_data = er32(FEXTNVM7);
> +	mac_data |= BIT(22);
> +	ew32(FEXTNVM7, mac_data);
> +
> +	/* Don't wake from dynamic Power Gating with clock request */
> +	mac_data = er32(FEXTNVM12);
> +	mac_data |= BIT(12);
> +	ew32(FEXTNVM12, mac_data);
> +
> +	/* Ungate PGCB clock */
> +	mac_data = er32(FEXTNVM9);
> +	mac_data &= ~BIT(28);
> +	ew32(FEXTNVM9, mac_data);
> +
> +	/* Enable K1 off to enable mPHY Power Gating */
> +	mac_data = er32(FEXTNVM6);
> +	mac_data |= BIT(31);
> +	ew32(FEXTNVM6, mac_data);
> +
> +	/* Enable mPHY power gating for any link and speed */
> +	mac_data = er32(FEXTNVM8);
> +	mac_data |= BIT(9);
> +	ew32(FEXTNVM8, mac_data);
> +
> +	/* No MAC DPG gating SLP_S0 in modern standby
> +	 * Switch the logic of the lanphypc to use PMC counter
> +	 */
> +	mac_data = er32(FEXTNVM5);
> +	mac_data |= BIT(7);
> +	ew32(FEXTNVM5, mac_data);
> +
>   	/* Disable the time synchronization clock */
>   	mac_data = er32(FEXTNVM7);
>   	mac_data |= BIT(31);
> @@ -6498,33 +6498,6 @@ static void e1000e_s0ix_exit_flow(struct e1000_adapter *adapter)
>   	} else {
>   		/* Request driver unconfigure the device from S0ix */
>   
> -		/* Disable the Dynamic Power Gating in the MAC */
> -		mac_data = er32(FEXTNVM7);
> -		mac_data &= 0xFFBFFFFF;
> -		ew32(FEXTNVM7, mac_data);
> -
> -		/* Disable mPHY power gating for any link and speed */
> -		mac_data = er32(FEXTNVM8);
> -		mac_data &= ~BIT(9);
> -		ew32(FEXTNVM8, mac_data);
> -
> -		/* Disable K1 off */
> -		mac_data = er32(FEXTNVM6);
> -		mac_data &= ~BIT(31);
> -		ew32(FEXTNVM6, mac_data);
> -
> -		/* Disable Ungate PGCB clock */
> -		mac_data = er32(FEXTNVM9);
> -		mac_data |= BIT(28);
> -		ew32(FEXTNVM9, mac_data);
> -
> -		/* Cancel not waking from dynamic
> -		 * Power Gating with clock request
> -		 */
> -		mac_data = er32(FEXTNVM12);
> -		mac_data &= ~BIT(12);
> -		ew32(FEXTNVM12, mac_data);
> -
>   		/* Cancel disable disconnected cable conditioning
>   		 * for Power Gating
>   		 */
> @@ -6537,13 +6510,6 @@ static void e1000e_s0ix_exit_flow(struct e1000_adapter *adapter)
>   		mac_data &= 0xFFF7FFFF;
>   		ew32(CTRL_EXT, mac_data);
>   
> -		/* Revert the lanphypc logic to use the internal Gbe counter
> -		 * and not the PMC counter
> -		 */
> -		mac_data = er32(FEXTNVM5);
> -		mac_data &= 0xFFFFFF7F;
> -		ew32(FEXTNVM5, mac_data);
> -
>   		/* Enable the periodic inband message,
>   		 * Request PCIe clock in K1 page770_17[10:9] =01b
>   		 */
> @@ -6581,6 +6547,40 @@ static void e1000e_s0ix_exit_flow(struct e1000_adapter *adapter)
>   	mac_data &= ~BIT(31);
>   	mac_data |= BIT(0);
>   	ew32(FEXTNVM7, mac_data);
> +
> +	/* Disable the Dynamic Power Gating in the MAC */
> +	mac_data = er32(FEXTNVM7);
> +	mac_data &= 0xFFBFFFFF;
> +	ew32(FEXTNVM7, mac_data);
> +
> +	/* Disable mPHY power gating for any link and speed */
> +	mac_data = er32(FEXTNVM8);
> +	mac_data &= ~BIT(9);
> +	ew32(FEXTNVM8, mac_data);
> +
> +	/* Disable K1 off */
> +	mac_data = er32(FEXTNVM6);
> +	mac_data &= ~BIT(31);
> +	ew32(FEXTNVM6, mac_data);
> +
> +	/* Disable Ungate PGCB clock */
> +	mac_data = er32(FEXTNVM9);
> +	mac_data |= BIT(28);
> +	ew32(FEXTNVM9, mac_data);
> +
> +	/* Cancel not waking from dynamic
> +	 * Power Gating with clock request
> +	 */
> +	mac_data = er32(FEXTNVM12);
> +	mac_data &= ~BIT(12);
> +	ew32(FEXTNVM12, mac_data);
> +
> +	/* Revert the lanphypc logic to use the internal Gbe counter
> +	 * and not the PMC counter
> +	 */
> +	mac_data = er32(FEXTNVM5);
> +	mac_data &= 0xFFFFFF7F;
> +	ew32(FEXTNVM5, mac_data);
>   }
>   
>   static int e1000e_pm_freeze(struct device *dev)

Doesn’t moving this out of the branch and running it unconditionally 
affect a lot more systems than Meteor Lake?


Kind regards,

Paul

Re: [Intel-wired-lan] [PATCH iwl-net v1 1/1] e1000e: Fix S0ix residency on Meteor Lake corporate systems

[Jacob Keller]

On 5/28/2024 3:43 AM, Paul Menzel wrote:
> Dear Vitaly,
> 
> 
> Thank you for the patch.
> 
> Am 28.05.24 um 12:33 schrieb Vitaly Lifshits:
>> From: Dima Ruinskiy <dima.ruinskiy@intel.com>
>>
>> On vPro systems,the  configuration of the I219-LM to achieve power
> 
> s/,the  /, the /
> 
>> gating and S0ix residency is split between the driver and the CSME FW.
>> It was discovered that in some scenarios, where the network cable is
>> connected and then disconnected, S0ix residency is not always reached.
> 
> Disconnected at any point, or just during suspend?
> 
> Any URL to the reports?
> 
>> This was root-caused to a subset of I219-LM register writes that are not
>> performed by the CSME FW. Therefore, the driver should perform these
>> register writes on corporate setups, regardless of the CSME FW state.
> 
> Is that documented somewhere?
> 
> Please add more information about the affected systems, and the test 
> environment (firmware versions, …).
> 
>> Fixes: cc23f4f0b6b9 ("e1000e: Add support for Meteor Lake")
>> Signed-off-by: Dima Ruinskiy <dima.ruinskiy@intel.com>
>> Signed-off-by: Vitaly.Lifshits <vitaly.lifshits@intel.com>
> 
> The line above with the dot can be removed.
> 
>> Signed-off-by: Vitaly Lifshits <vitaly.lifshits@intel.com>
>> ---

I would also appreciate clarifications so we can understand the fix.

>>   drivers/net/ethernet/intel/e1000e/netdev.c | 132 ++++++++++-----------
>>   1 file changed, 66 insertions(+), 66 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
>> index da5c59daf8ba..3cd161c6672b 100644
>> --- a/drivers/net/ethernet/intel/e1000e/netdev.c
>> +++ b/drivers/net/ethernet/intel/e1000e/netdev.c
>> @@ -6363,49 +6363,49 @@ static void e1000e_s0ix_entry_flow(struct e1000_adapter *adapter)
>>   		mac_data |= E1000_EXTCNF_CTRL_GATE_PHY_CFG;
>>   		ew32(EXTCNF_CTRL, mac_data);
>>   
>> -		/* Enable the Dynamic Power Gating in the MAC */
>> -		mac_data = er32(FEXTNVM7);
>> -		mac_data |= BIT(22);
>> -		ew32(FEXTNVM7, mac_data);
>> -
>>   		/* Disable disconnected cable conditioning for Power Gating */
>>   		mac_data = er32(DPGFR);
>>   		mac_data |= BIT(2);
>>   		ew32(DPGFR, mac_data);
>>   
>> -		/* Don't wake from dynamic Power Gating with clock request */
>> -		mac_data = er32(FEXTNVM12);
>> -		mac_data |= BIT(12);
>> -		ew32(FEXTNVM12, mac_data);
>> -
>> -		/* Ungate PGCB clock */
>> -		mac_data = er32(FEXTNVM9);
>> -		mac_data &= ~BIT(28);
>> -		ew32(FEXTNVM9, mac_data);
>> -
>> -		/* Enable K1 off to enable mPHY Power Gating */
>> -		mac_data = er32(FEXTNVM6);
>> -		mac_data |= BIT(31);
>> -		ew32(FEXTNVM6, mac_data);
>> -
>> -		/* Enable mPHY power gating for any link and speed */
>> -		mac_data = er32(FEXTNVM8);
>> -		mac_data |= BIT(9);
>> -		ew32(FEXTNVM8, mac_data);
>> -
>>   		/* Enable the Dynamic Clock Gating in the DMA and MAC */
>>   		mac_data = er32(CTRL_EXT);
>>   		mac_data |= E1000_CTRL_EXT_DMA_DYN_CLK_EN;
>>   		ew32(CTRL_EXT, mac_data);
>> -
>> -		/* No MAC DPG gating SLP_S0 in modern standby
>> -		 * Switch the logic of the lanphypc to use PMC counter
>> -		 */
>> -		mac_data = er32(FEXTNVM5);
>> -		mac_data |= BIT(7);
>> -		ew32(FEXTNVM5, mac_data);
>>   	}
>>   
>> +	/* Enable the Dynamic Power Gating in the MAC */
>> +	mac_data = er32(FEXTNVM7);
>> +	mac_data |= BIT(22);
>> +	ew32(FEXTNVM7, mac_data);
>> +
>> +	/* Don't wake from dynamic Power Gating with clock request */
>> +	mac_data = er32(FEXTNVM12);
>> +	mac_data |= BIT(12);
>> +	ew32(FEXTNVM12, mac_data);
>> +
>> +	/* Ungate PGCB clock */
>> +	mac_data = er32(FEXTNVM9);
>> +	mac_data &= ~BIT(28);
>> +	ew32(FEXTNVM9, mac_data);
>> +
>> +	/* Enable K1 off to enable mPHY Power Gating */
>> +	mac_data = er32(FEXTNVM6);
>> +	mac_data |= BIT(31);
>> +	ew32(FEXTNVM6, mac_data);
>> +
>> +	/* Enable mPHY power gating for any link and speed */
>> +	mac_data = er32(FEXTNVM8);
>> +	mac_data |= BIT(9);
>> +	ew32(FEXTNVM8, mac_data);
>> +
>> +	/* No MAC DPG gating SLP_S0 in modern standby
>> +	 * Switch the logic of the lanphypc to use PMC counter
>> +	 */
>> +	mac_data = er32(FEXTNVM5);
>> +	mac_data |= BIT(7);
>> +	ew32(FEXTNVM5, mac_data);
>> +
>>   	/* Disable the time synchronization clock */
>>   	mac_data = er32(FEXTNVM7);
>>   	mac_data |= BIT(31);
>> @@ -6498,33 +6498,6 @@ static void e1000e_s0ix_exit_flow(struct e1000_adapter *adapter)
>>   	} else {
>>   		/* Request driver unconfigure the device from S0ix */
>>   
>> -		/* Disable the Dynamic Power Gating in the MAC */
>> -		mac_data = er32(FEXTNVM7);
>> -		mac_data &= 0xFFBFFFFF;
>> -		ew32(FEXTNVM7, mac_data);
>> -
>> -		/* Disable mPHY power gating for any link and speed */
>> -		mac_data = er32(FEXTNVM8);
>> -		mac_data &= ~BIT(9);
>> -		ew32(FEXTNVM8, mac_data);
>> -
>> -		/* Disable K1 off */
>> -		mac_data = er32(FEXTNVM6);
>> -		mac_data &= ~BIT(31);
>> -		ew32(FEXTNVM6, mac_data);
>> -
>> -		/* Disable Ungate PGCB clock */
>> -		mac_data = er32(FEXTNVM9);
>> -		mac_data |= BIT(28);
>> -		ew32(FEXTNVM9, mac_data);
>> -
>> -		/* Cancel not waking from dynamic
>> -		 * Power Gating with clock request
>> -		 */
>> -		mac_data = er32(FEXTNVM12);
>> -		mac_data &= ~BIT(12);
>> -		ew32(FEXTNVM12, mac_data);
>> -
>>   		/* Cancel disable disconnected cable conditioning
>>   		 * for Power Gating
>>   		 */
>> @@ -6537,13 +6510,6 @@ static void e1000e_s0ix_exit_flow(struct e1000_adapter *adapter)
>>   		mac_data &= 0xFFF7FFFF;
>>   		ew32(CTRL_EXT, mac_data);
>>   
>> -		/* Revert the lanphypc logic to use the internal Gbe counter
>> -		 * and not the PMC counter
>> -		 */
>> -		mac_data = er32(FEXTNVM5);
>> -		mac_data &= 0xFFFFFF7F;
>> -		ew32(FEXTNVM5, mac_data);
>> -
>>   		/* Enable the periodic inband message,
>>   		 * Request PCIe clock in K1 page770_17[10:9] =01b
>>   		 */
>> @@ -6581,6 +6547,40 @@ static void e1000e_s0ix_exit_flow(struct e1000_adapter *adapter)
>>   	mac_data &= ~BIT(31);
>>   	mac_data |= BIT(0);
>>   	ew32(FEXTNVM7, mac_data);
>> +
>> +	/* Disable the Dynamic Power Gating in the MAC */
>> +	mac_data = er32(FEXTNVM7);
>> +	mac_data &= 0xFFBFFFFF;
>> +	ew32(FEXTNVM7, mac_data);
>> +
>> +	/* Disable mPHY power gating for any link and speed */
>> +	mac_data = er32(FEXTNVM8);
>> +	mac_data &= ~BIT(9);
>> +	ew32(FEXTNVM8, mac_data);
>> +
>> +	/* Disable K1 off */
>> +	mac_data = er32(FEXTNVM6);
>> +	mac_data &= ~BIT(31);
>> +	ew32(FEXTNVM6, mac_data);
>> +
>> +	/* Disable Ungate PGCB clock */
>> +	mac_data = er32(FEXTNVM9);
>> +	mac_data |= BIT(28);
>> +	ew32(FEXTNVM9, mac_data);
>> +
>> +	/* Cancel not waking from dynamic
>> +	 * Power Gating with clock request
>> +	 */
>> +	mac_data = er32(FEXTNVM12);
>> +	mac_data &= ~BIT(12);
>> +	ew32(FEXTNVM12, mac_data);
>> +
>> +	/* Revert the lanphypc logic to use the internal Gbe counter
>> +	 * and not the PMC counter
>> +	 */
>> +	mac_data = er32(FEXTNVM5);
>> +	mac_data &= 0xFFFFFF7F;
>> +	ew32(FEXTNVM5, mac_data);
>>   }
>>   
>>   static int e1000e_pm_freeze(struct device *dev)
> 
> Doesn’t moving this out of the branch and running it unconditionally 
> affect a lot more systems than Meteor Lake?
> 
> 

I would like an explanation of this as well.

Thanks,
Jake

> Kind regards,
> 
> Paul

Re: [Intel-wired-lan] [PATCH iwl-net v1 1/1] e1000e: Fix S0ix residency on Meteor Lake corporate systems

[Lifshits, Vitaly]

On 5/28/2024 1:43 PM, Paul Menzel wrote:
> Dear Vitaly,
> 
> 
> Thank you for the patch.
> 
> Am 28.05.24 um 12:33 schrieb Vitaly Lifshits:
>> From: Dima Ruinskiy <dima.ruinskiy@intel.com>
>>
>> On vPro systems,the  configuration of the I219-LM to achieve power
> 
> s/,the  /, the /
Thank you for noticing it.
I will fix it in a v2.
> 
>> gating and S0ix residency is split between the driver and the CSME FW.
>> It was discovered that in some scenarios, where the network cable is
>> connected and then disconnected, S0ix residency is not always reached.
> 
> Disconnected at any point, or just during suspend?
Yes, at any point.
> 
> Any URL to the reports?
Yes, https://bugs.launchpad.net/sutton/+bug/2054657
> 
>> This was root-caused to a subset of I219-LM register writes that are not
>> performed by the CSME FW. Therefore, the driver should perform these
>> register writes on corporate setups, regardless of the CSME FW state.
> 
> Is that documented somewhere?
Only in an internal documentation.
> 
> Please add more information about the affected systems, and the test 
> environment (firmware versions, …).
It is mentioned at the beginning of the commit, Meteorlake vPro systems.

> 
>> Fixes: cc23f4f0b6b9 ("e1000e: Add support for Meteor Lake")
>> Signed-off-by: Dima Ruinskiy <dima.ruinskiy@intel.com>
>> Signed-off-by: Vitaly.Lifshits <vitaly.lifshits@intel.com>
> 
> The line above with the dot can be removed.
Will be fixed in a v2.
> 
>> Signed-off-by: Vitaly Lifshits <vitaly.lifshits@intel.com>
>> ---
>>   drivers/net/ethernet/intel/e1000e/netdev.c | 132 ++++++++++-----------
>>   1 file changed, 66 insertions(+), 66 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c 
>> b/drivers/net/ethernet/intel/e1000e/netdev.c
>> index da5c59daf8ba..3cd161c6672b 100644
>> --- a/drivers/net/ethernet/intel/e1000e/netdev.c
>> +++ b/drivers/net/ethernet/intel/e1000e/netdev.c
>> @@ -6363,49 +6363,49 @@ static void e1000e_s0ix_entry_flow(struct 
>> e1000_adapter *adapter)
>>           mac_data |= E1000_EXTCNF_CTRL_GATE_PHY_CFG;
>>           ew32(EXTCNF_CTRL, mac_data);
>> -        /* Enable the Dynamic Power Gating in the MAC */
>> -        mac_data = er32(FEXTNVM7);
>> -        mac_data |= BIT(22);
>> -        ew32(FEXTNVM7, mac_data);
>> -
>>           /* Disable disconnected cable conditioning for Power Gating */
>>           mac_data = er32(DPGFR);
>>           mac_data |= BIT(2);
>>           ew32(DPGFR, mac_data);
>> -        /* Don't wake from dynamic Power Gating with clock request */
>> -        mac_data = er32(FEXTNVM12);
>> -        mac_data |= BIT(12);
>> -        ew32(FEXTNVM12, mac_data);
>> -
>> -        /* Ungate PGCB clock */
>> -        mac_data = er32(FEXTNVM9);
>> -        mac_data &= ~BIT(28);
>> -        ew32(FEXTNVM9, mac_data);
>> -
>> -        /* Enable K1 off to enable mPHY Power Gating */
>> -        mac_data = er32(FEXTNVM6);
>> -        mac_data |= BIT(31);
>> -        ew32(FEXTNVM6, mac_data);
>> -
>> -        /* Enable mPHY power gating for any link and speed */
>> -        mac_data = er32(FEXTNVM8);
>> -        mac_data |= BIT(9);
>> -        ew32(FEXTNVM8, mac_data);
>> -
>>           /* Enable the Dynamic Clock Gating in the DMA and MAC */
>>           mac_data = er32(CTRL_EXT);
>>           mac_data |= E1000_CTRL_EXT_DMA_DYN_CLK_EN;
>>           ew32(CTRL_EXT, mac_data);
>> -
>> -        /* No MAC DPG gating SLP_S0 in modern standby
>> -         * Switch the logic of the lanphypc to use PMC counter
>> -         */
>> -        mac_data = er32(FEXTNVM5);
>> -        mac_data |= BIT(7);
>> -        ew32(FEXTNVM5, mac_data);
>>       }
>> +    /* Enable the Dynamic Power Gating in the MAC */
>> +    mac_data = er32(FEXTNVM7);
>> +    mac_data |= BIT(22);
>> +    ew32(FEXTNVM7, mac_data);
>> +
>> +    /* Don't wake from dynamic Power Gating with clock request */
>> +    mac_data = er32(FEXTNVM12);
>> +    mac_data |= BIT(12);
>> +    ew32(FEXTNVM12, mac_data);
>> +
>> +    /* Ungate PGCB clock */
>> +    mac_data = er32(FEXTNVM9);
>> +    mac_data &= ~BIT(28);
>> +    ew32(FEXTNVM9, mac_data);
>> +
>> +    /* Enable K1 off to enable mPHY Power Gating */
>> +    mac_data = er32(FEXTNVM6);
>> +    mac_data |= BIT(31);
>> +    ew32(FEXTNVM6, mac_data);
>> +
>> +    /* Enable mPHY power gating for any link and speed */
>> +    mac_data = er32(FEXTNVM8);
>> +    mac_data |= BIT(9);
>> +    ew32(FEXTNVM8, mac_data);
>> +
>> +    /* No MAC DPG gating SLP_S0 in modern standby
>> +     * Switch the logic of the lanphypc to use PMC counter
>> +     */
>> +    mac_data = er32(FEXTNVM5);
>> +    mac_data |= BIT(7);
>> +    ew32(FEXTNVM5, mac_data);
>> +
>>       /* Disable the time synchronization clock */
>>       mac_data = er32(FEXTNVM7);
>>       mac_data |= BIT(31);
>> @@ -6498,33 +6498,6 @@ static void e1000e_s0ix_exit_flow(struct 
>> e1000_adapter *adapter)
>>       } else {
>>           /* Request driver unconfigure the device from S0ix */
>> -        /* Disable the Dynamic Power Gating in the MAC */
>> -        mac_data = er32(FEXTNVM7);
>> -        mac_data &= 0xFFBFFFFF;
>> -        ew32(FEXTNVM7, mac_data);
>> -
>> -        /* Disable mPHY power gating for any link and speed */
>> -        mac_data = er32(FEXTNVM8);
>> -        mac_data &= ~BIT(9);
>> -        ew32(FEXTNVM8, mac_data);
>> -
>> -        /* Disable K1 off */
>> -        mac_data = er32(FEXTNVM6);
>> -        mac_data &= ~BIT(31);
>> -        ew32(FEXTNVM6, mac_data);
>> -
>> -        /* Disable Ungate PGCB clock */
>> -        mac_data = er32(FEXTNVM9);
>> -        mac_data |= BIT(28);
>> -        ew32(FEXTNVM9, mac_data);
>> -
>> -        /* Cancel not waking from dynamic
>> -         * Power Gating with clock request
>> -         */
>> -        mac_data = er32(FEXTNVM12);
>> -        mac_data &= ~BIT(12);
>> -        ew32(FEXTNVM12, mac_data);
>> -
>>           /* Cancel disable disconnected cable conditioning
>>            * for Power Gating
>>            */
>> @@ -6537,13 +6510,6 @@ static void e1000e_s0ix_exit_flow(struct 
>> e1000_adapter *adapter)
>>           mac_data &= 0xFFF7FFFF;
>>           ew32(CTRL_EXT, mac_data);
>> -        /* Revert the lanphypc logic to use the internal Gbe counter
>> -         * and not the PMC counter
>> -         */
>> -        mac_data = er32(FEXTNVM5);
>> -        mac_data &= 0xFFFFFF7F;
>> -        ew32(FEXTNVM5, mac_data);
>> -
>>           /* Enable the periodic inband message,
>>            * Request PCIe clock in K1 page770_17[10:9] =01b
>>            */
>> @@ -6581,6 +6547,40 @@ static void e1000e_s0ix_exit_flow(struct 
>> e1000_adapter *adapter)
>>       mac_data &= ~BIT(31);
>>       mac_data |= BIT(0);
>>       ew32(FEXTNVM7, mac_data);
>> +
>> +    /* Disable the Dynamic Power Gating in the MAC */
>> +    mac_data = er32(FEXTNVM7);
>> +    mac_data &= 0xFFBFFFFF;
>> +    ew32(FEXTNVM7, mac_data);
>> +
>> +    /* Disable mPHY power gating for any link and speed */
>> +    mac_data = er32(FEXTNVM8);
>> +    mac_data &= ~BIT(9);
>> +    ew32(FEXTNVM8, mac_data);
>> +
>> +    /* Disable K1 off */
>> +    mac_data = er32(FEXTNVM6);
>> +    mac_data &= ~BIT(31);
>> +    ew32(FEXTNVM6, mac_data);
>> +
>> +    /* Disable Ungate PGCB clock */
>> +    mac_data = er32(FEXTNVM9);
>> +    mac_data |= BIT(28);
>> +    ew32(FEXTNVM9, mac_data);
>> +
>> +    /* Cancel not waking from dynamic
>> +     * Power Gating with clock request
>> +     */
>> +    mac_data = er32(FEXTNVM12);
>> +    mac_data &= ~BIT(12);
>> +    ew32(FEXTNVM12, mac_data);
>> +
>> +    /* Revert the lanphypc logic to use the internal Gbe counter
>> +     * and not the PMC counter
>> +     */
>> +    mac_data = er32(FEXTNVM5);
>> +    mac_data &= 0xFFFFFF7F;
>> +    ew32(FEXTNVM5, mac_data);
>>   }
>>   static int e1000e_pm_freeze(struct device *dev)
> 
> Doesn’t moving this out of the branch and running it unconditionally 
> affect a lot more systems than Meteor Lake?
> 
> 
> Kind regards,
> 
> Paul

Re: [Intel-wired-lan] [PATCH iwl-net v1 1/1] e1000e: Fix S0ix residency on Meteor Lake corporate systems

[Paul Menzel]

Dear Vitaly,


Thank you for your reply.

Am 29.05.24 um 13:13 schrieb Lifshits, Vitaly:

> On 5/28/2024 1:43 PM, Paul Menzel wrote:

>> Am 28.05.24 um 12:33 schrieb Vitaly Lifshits:
>>> From: Dima Ruinskiy <dima.ruinskiy@intel.com>
>>>
>>> On vPro systems,the  configuration of the I219-LM to achieve power
>>
>> s/,the  /, the /
> Thank you for noticing it.
> I will fix it in a v2.
>
>>> gating and S0ix residency is split between the driver and the CSME FW.
>>> It was discovered that in some scenarios, where the network cable is
>>> connected and then disconnected, S0ix residency is not always reached.
>>
>> Disconnected at any point, or just during suspend?
> Yes, at any point.
>
>> Any URL to the reports?
> Yes, https://bugs.launchpad.net/sutton/+bug/2054657

This page does not exist.

     $ curl -I https://bugs.launchpad.net/sutton/+bug/2054657
     HTTP/1.1 404 Not Found
     Date: Wed, 29 May 2024 11:20:52 GMT
     Server: gunicorn
     […]

Please reference the (working) URL in a Link: tag in the footer.

>>> This was root-caused to a subset of I219-LM register writes that are not
>>> performed by the CSME FW. Therefore, the driver should perform these
>>> register writes on corporate setups, regardless of the CSME FW state.
>>
>> Is that documented somewhere?
> Only in an internal documentation.

If you can, it’d be great if you summarized why this is triggered by 
unplugging the network cable.

>> Please add more information about the affected systems, and the test 
>> environment (firmware versions, …).
> It is mentioned at the beginning of the commit, Meteorlake vPro systems.

Please be more specific. If there is a Meteor Lake vPro system, where 
this can *not* be reproduced, it’d be great to know the exact system you 
tested this with. Meteor Lake has been released a long time ago, and not 
remembering a lot of bug reports, I assume, it’s not always reproducible.

(You missed to comment on my previous comments at the end.)

>>> Fixes: cc23f4f0b6b9 ("e1000e: Add support for Meteor Lake")
>>> Signed-off-by: Dima Ruinskiy <dima.ruinskiy@intel.com>
>>> Signed-off-by: Vitaly.Lifshits <vitaly.lifshits@intel.com>
>>
>> The line above with the dot can be removed.
> Will be fixed in a v2.
>>
>>> Signed-off-by: Vitaly Lifshits <vitaly.lifshits@intel.com>
>>> ---
>>>   drivers/net/ethernet/intel/e1000e/netdev.c | 132 ++++++++++-----------
>>>   1 file changed, 66 insertions(+), 66 deletions(-)
>>>
>>> diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c 
>>> b/drivers/net/ethernet/intel/e1000e/netdev.c
>>> index da5c59daf8ba..3cd161c6672b 100644
>>> --- a/drivers/net/ethernet/intel/e1000e/netdev.c
>>> +++ b/drivers/net/ethernet/intel/e1000e/netdev.c
>>> @@ -6363,49 +6363,49 @@ static void e1000e_s0ix_entry_flow(struct 
>>> e1000_adapter *adapter)
>>>           mac_data |= E1000_EXTCNF_CTRL_GATE_PHY_CFG;
>>>           ew32(EXTCNF_CTRL, mac_data);
>>> -        /* Enable the Dynamic Power Gating in the MAC */
>>> -        mac_data = er32(FEXTNVM7);
>>> -        mac_data |= BIT(22);
>>> -        ew32(FEXTNVM7, mac_data);
>>> -
>>>           /* Disable disconnected cable conditioning for Power Gating */
>>>           mac_data = er32(DPGFR);
>>>           mac_data |= BIT(2);
>>>           ew32(DPGFR, mac_data);
>>> -        /* Don't wake from dynamic Power Gating with clock request */
>>> -        mac_data = er32(FEXTNVM12);
>>> -        mac_data |= BIT(12);
>>> -        ew32(FEXTNVM12, mac_data);
>>> -
>>> -        /* Ungate PGCB clock */
>>> -        mac_data = er32(FEXTNVM9);
>>> -        mac_data &= ~BIT(28);
>>> -        ew32(FEXTNVM9, mac_data);
>>> -
>>> -        /* Enable K1 off to enable mPHY Power Gating */
>>> -        mac_data = er32(FEXTNVM6);
>>> -        mac_data |= BIT(31);
>>> -        ew32(FEXTNVM6, mac_data);
>>> -
>>> -        /* Enable mPHY power gating for any link and speed */
>>> -        mac_data = er32(FEXTNVM8);
>>> -        mac_data |= BIT(9);
>>> -        ew32(FEXTNVM8, mac_data);
>>> -
>>>           /* Enable the Dynamic Clock Gating in the DMA and MAC */
>>>           mac_data = er32(CTRL_EXT);
>>>           mac_data |= E1000_CTRL_EXT_DMA_DYN_CLK_EN;
>>>           ew32(CTRL_EXT, mac_data);
>>> -
>>> -        /* No MAC DPG gating SLP_S0 in modern standby
>>> -         * Switch the logic of the lanphypc to use PMC counter
>>> -         */
>>> -        mac_data = er32(FEXTNVM5);
>>> -        mac_data |= BIT(7);
>>> -        ew32(FEXTNVM5, mac_data);
>>>       }
>>> +    /* Enable the Dynamic Power Gating in the MAC */
>>> +    mac_data = er32(FEXTNVM7);
>>> +    mac_data |= BIT(22);
>>> +    ew32(FEXTNVM7, mac_data);
>>> +
>>> +    /* Don't wake from dynamic Power Gating with clock request */
>>> +    mac_data = er32(FEXTNVM12);
>>> +    mac_data |= BIT(12);
>>> +    ew32(FEXTNVM12, mac_data);
>>> +
>>> +    /* Ungate PGCB clock */
>>> +    mac_data = er32(FEXTNVM9);
>>> +    mac_data &= ~BIT(28);
>>> +    ew32(FEXTNVM9, mac_data);
>>> +
>>> +    /* Enable K1 off to enable mPHY Power Gating */
>>> +    mac_data = er32(FEXTNVM6);
>>> +    mac_data |= BIT(31);
>>> +    ew32(FEXTNVM6, mac_data);
>>> +
>>> +    /* Enable mPHY power gating for any link and speed */
>>> +    mac_data = er32(FEXTNVM8);
>>> +    mac_data |= BIT(9);
>>> +    ew32(FEXTNVM8, mac_data);
>>> +
>>> +    /* No MAC DPG gating SLP_S0 in modern standby
>>> +     * Switch the logic of the lanphypc to use PMC counter
>>> +     */
>>> +    mac_data = er32(FEXTNVM5);
>>> +    mac_data |= BIT(7);
>>> +    ew32(FEXTNVM5, mac_data);
>>> +
>>>       /* Disable the time synchronization clock */
>>>       mac_data = er32(FEXTNVM7);
>>>       mac_data |= BIT(31);
>>> @@ -6498,33 +6498,6 @@ static void e1000e_s0ix_exit_flow(struct 
>>> e1000_adapter *adapter)
>>>       } else {
>>>           /* Request driver unconfigure the device from S0ix */
>>> -        /* Disable the Dynamic Power Gating in the MAC */
>>> -        mac_data = er32(FEXTNVM7);
>>> -        mac_data &= 0xFFBFFFFF;
>>> -        ew32(FEXTNVM7, mac_data);
>>> -
>>> -        /* Disable mPHY power gating for any link and speed */
>>> -        mac_data = er32(FEXTNVM8);
>>> -        mac_data &= ~BIT(9);
>>> -        ew32(FEXTNVM8, mac_data);
>>> -
>>> -        /* Disable K1 off */
>>> -        mac_data = er32(FEXTNVM6);
>>> -        mac_data &= ~BIT(31);
>>> -        ew32(FEXTNVM6, mac_data);
>>> -
>>> -        /* Disable Ungate PGCB clock */
>>> -        mac_data = er32(FEXTNVM9);
>>> -        mac_data |= BIT(28);
>>> -        ew32(FEXTNVM9, mac_data);
>>> -
>>> -        /* Cancel not waking from dynamic
>>> -         * Power Gating with clock request
>>> -         */
>>> -        mac_data = er32(FEXTNVM12);
>>> -        mac_data &= ~BIT(12);
>>> -        ew32(FEXTNVM12, mac_data);
>>> -
>>>           /* Cancel disable disconnected cable conditioning
>>>            * for Power Gating
>>>            */
>>> @@ -6537,13 +6510,6 @@ static void e1000e_s0ix_exit_flow(struct 
>>> e1000_adapter *adapter)
>>>           mac_data &= 0xFFF7FFFF;
>>>           ew32(CTRL_EXT, mac_data);
>>> -        /* Revert the lanphypc logic to use the internal Gbe counter
>>> -         * and not the PMC counter
>>> -         */
>>> -        mac_data = er32(FEXTNVM5);
>>> -        mac_data &= 0xFFFFFF7F;
>>> -        ew32(FEXTNVM5, mac_data);
>>> -
>>>           /* Enable the periodic inband message,
>>>            * Request PCIe clock in K1 page770_17[10:9] =01b
>>>            */
>>> @@ -6581,6 +6547,40 @@ static void e1000e_s0ix_exit_flow(struct 
>>> e1000_adapter *adapter)
>>>       mac_data &= ~BIT(31);
>>>       mac_data |= BIT(0);
>>>       ew32(FEXTNVM7, mac_data);
>>> +
>>> +    /* Disable the Dynamic Power Gating in the MAC */
>>> +    mac_data = er32(FEXTNVM7);
>>> +    mac_data &= 0xFFBFFFFF;
>>> +    ew32(FEXTNVM7, mac_data);
>>> +
>>> +    /* Disable mPHY power gating for any link and speed */
>>> +    mac_data = er32(FEXTNVM8);
>>> +    mac_data &= ~BIT(9);
>>> +    ew32(FEXTNVM8, mac_data);
>>> +
>>> +    /* Disable K1 off */
>>> +    mac_data = er32(FEXTNVM6);
>>> +    mac_data &= ~BIT(31);
>>> +    ew32(FEXTNVM6, mac_data);
>>> +
>>> +    /* Disable Ungate PGCB clock */
>>> +    mac_data = er32(FEXTNVM9);
>>> +    mac_data |= BIT(28);
>>> +    ew32(FEXTNVM9, mac_data);
>>> +
>>> +    /* Cancel not waking from dynamic
>>> +     * Power Gating with clock request
>>> +     */
>>> +    mac_data = er32(FEXTNVM12);
>>> +    mac_data &= ~BIT(12);
>>> +    ew32(FEXTNVM12, mac_data);
>>> +
>>> +    /* Revert the lanphypc logic to use the internal Gbe counter
>>> +     * and not the PMC counter
>>> +     */
>>> +    mac_data = er32(FEXTNVM5);
>>> +    mac_data &= 0xFFFFFF7F;
>>> +    ew32(FEXTNVM5, mac_data);
>>>   }
>>>   static int e1000e_pm_freeze(struct device *dev)
>>
>> Doesn’t moving this out of the branch and running it unconditionally 
>> affect a lot more systems than Meteor Lake?


Kind regards,

Paul

Re: [Intel-wired-lan] [PATCH iwl-net v1 1/1] e1000e: Fix S0ix residency on Meteor Lake corporate systems

[Mark Pearson]

Hi Paul

On Wed, May 29, 2024, at 7:23 AM, Paul Menzel wrote:
> Dear Vitaly,
>
>
> Thank you for your reply.
>
> Am 29.05.24 um 13:13 schrieb Lifshits, Vitaly:
>
>> On 5/28/2024 1:43 PM, Paul Menzel wrote:
>
>>> Am 28.05.24 um 12:33 schrieb Vitaly Lifshits:
>>>> From: Dima Ruinskiy <dima.ruinskiy@intel.com>
>>>>
>>>> On vPro systems,the  configuration of the I219-LM to achieve power
>>>
>>> s/,the  /, the /
>> Thank you for noticing it.
>> I will fix it in a v2.
>>
>>>> gating and S0ix residency is split between the driver and the CSME FW.
>>>> It was discovered that in some scenarios, where the network cable is
>>>> connected and then disconnected, S0ix residency is not always reached.
>>>
>>> Disconnected at any point, or just during suspend?
>> Yes, at any point.
>>
>>> Any URL to the reports?
>> Yes, https://bugs.launchpad.net/sutton/+bug/2054657
>
> This page does not exist.
>
>      $ curl -I https://bugs.launchpad.net/sutton/+bug/2054657
>      HTTP/1.1 404 Not Found
>      Date: Wed, 29 May 2024 11:20:52 GMT
>      Server: gunicorn
>      […]
>
> Please reference the (working) URL in a Link: tag in the footer.
>

Unfortunately the URL is a private bug between Lenovo and Canonical - we hit the issue as we've been working on the Linux enablement for our 2024 platforms.
I can't make the bug publicly visible I'm afraid - too much internal code name/process in there.
I don't think there will be a public report of the issue yet - because the platforms have only just been released.
If it's useful I guess I can create a kernel.org bug to track against?

>>>> This was root-caused to a subset of I219-LM register writes that are not
>>>> performed by the CSME FW. Therefore, the driver should perform these
>>>> register writes on corporate setups, regardless of the CSME FW state.
>>>
>>> Is that documented somewhere?
>> Only in an internal documentation.
>
> If you can, it’d be great if you summarized why this is triggered by 
> unplugging the network cable.
>
>>> Please add more information about the affected systems, and the test 
>>> environment (firmware versions, …).
>> It is mentioned at the beginning of the commit, Meteorlake vPro systems.
>
> Please be more specific. If there is a Meteor Lake vPro system, where 
> this can *not* be reproduced, it’d be great to know the exact system you 
> tested this with. Meteor Lake has been released a long time ago, and not 
> remembering a lot of bug reports, I assume, it’s not always reproducible.
>
> (You missed to comment on my previous comments at the end.)

I can't comment on the technical details, but I can note that it's reproduced on multiple Lenovo Thinkpad Meteorlake platforms that have ethernet
E14 G6, E16 G2, L14 G5, L16 G1, P14s G5, P16s G3 & P16v G2
I think it's common to any Meteorlake platform - but I defer to the Intel folk on the specifics.

Mark (tech lead for Lenovo PC Linux team)

Re: [Intel-wired-lan] [PATCH iwl-net v1 1/1] e1000e: Fix S0ix residency on Meteor Lake corporate systems

[Paul Menzel]

Dear Mark,


Am 04.06.24 um 16:33 schrieb Mark Pearson:

> On Wed, May 29, 2024, at 7:23 AM, Paul Menzel wrote:

>> Am 29.05.24 um 13:13 schrieb Lifshits, Vitaly:
>>
>>> On 5/28/2024 1:43 PM, Paul Menzel wrote:
>>
>>>> Am 28.05.24 um 12:33 schrieb Vitaly Lifshits:
>>>>> From: Dima Ruinskiy <dima.ruinskiy@intel.com>
>>>>>
>>>>> On vPro systems,the  configuration of the I219-LM to achieve power
>>>>
>>>> s/,the  /, the /
>>> Thank you for noticing it.
>>> I will fix it in a v2.
>>>
>>>>> gating and S0ix residency is split between the driver and the CSME FW.
>>>>> It was discovered that in some scenarios, where the network cable is
>>>>> connected and then disconnected, S0ix residency is not always reached.
>>>>
>>>> Disconnected at any point, or just during suspend?
>>> Yes, at any point.
>>>
>>>> Any URL to the reports?
>>> Yes, https://bugs.launchpad.net/sutton/+bug/2054657
>>
>> This page does not exist.
>>
>>       $ curl -I https://bugs.launchpad.net/sutton/+bug/2054657
>>       HTTP/1.1 404 Not Found
>>       Date: Wed, 29 May 2024 11:20:52 GMT
>>       Server: gunicorn
>>       […]
>>
>> Please reference the (working) URL in a Link: tag in the footer.
> 
> Unfortunately the URL is a private bug between Lenovo and Canonical -
> we hit the issue as we've been working on the Linux enablement for
> our 2024 platforms.

Thank you for the clarification.

> I can't make the bug publicly visible I'm afraid - too much internal
> code name/process in there. I don't think there will be a public
> report of the issue yet - because the platforms have only just been
> released. If it's useful I guess I can create a kernel.org bug to
> track against?
If all relevant information is in the commit message, then that’s not 
necessary. The private bug URL could still be mentioned, as it might be 
useful for those with access. It should only be clearly stated, that 
it’s not public. (Others have different opinion.)

>>>>> This was root-caused to a subset of I219-LM register writes that are not
>>>>> performed by the CSME FW. Therefore, the driver should perform these
>>>>> register writes on corporate setups, regardless of the CSME FW state.
>>>>
>>>> Is that documented somewhere?
>>> Only in an internal documentation.
>>
>> If you can, it’d be great if you summarized why this is triggered by
>> unplugging the network cable.
>>
>>>> Please add more information about the affected systems, and the test
>>>> environment (firmware versions, …).
>>> It is mentioned at the beginning of the commit, Meteorlake vPro systems.
>>
>> Please be more specific. If there is a Meteor Lake vPro system, where
>> this can *not* be reproduced, it’d be great to know the exact system you
>> tested this with. Meteor Lake has been released a long time ago, and not
>> remembering a lot of bug reports, I assume, it’s not always reproducible.
>>
>> (You missed to comment on my previous comments at the end.)
> 
> I can't comment on the technical details, but I can note that it's
> reproduced on multiple Lenovo Thinkpad Meteorlake platforms that have
> ethernet
> E14 G6, E16 G2, L14 G5, L16 G1, P14s G5, P16s G3 & P16v G2
> I think it's common to any Meteorlake platform - but I defer to the
> Intel folk on the specifics.
Adding these model names to the commit message would work for me.


Kind regards,

Paul