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

["Lifshits, Vitaly" <vitaly.lifshits@intel.com>]

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