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.ruins...@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.ruins...@intel.com>
>> Signed-off-by: Vitaly.Lifshits <vitaly.lifsh...@intel.com>
>
> The line above with the dot can be removed.
>
>> Signed-off-by: Vitaly Lifshits <vitaly.lifsh...@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
