[edk2-platforms][PATCH 0/3] Add necessary changes for PCIe core and remove unnecessary checks for PCIe Devmap mode

[edk2-platforms][PATCH 0/3] Add necessary changes for PCIe core and remove unnecessary checks for PCIe Devmap mode

[Minh Nguyen]

These patches help to improve PCIe core for Link training sequence and
add logic to check PCIe card is present or not. One patch also removes
unnecessary checks for PCIe Devmap mode.

Minh Nguyen (2):
  AmpereAltraPkg: Remove PCIe "DevMapModeAuto" case in retrieving Devmap
    mode
  AmpereAltraPkg: Fix PCIe link training sequence

Tinh Nguyen (1):
  AmpereAltraPkg: Add retry logic when PCIe card is present

 Silicon/Ampere/AmpereAltraPkg/Drivers/PcieInitPei/RootComplexNVParam.c |  2 +-
 Silicon/Ampere/AmpereAltraPkg/Library/Ac01PcieLib/PcieCore.c           | 12 ++++++++----
 2 files changed, 9 insertions(+), 5 deletions(-)

-- 
2.39.0

[edk2-platforms][PATCH 1/3] AmpereAltraPkg: Remove PCIe "DevMapModeAuto" case in retrieving Devmap mode

[Minh Nguyen]

Because PCIe Auto bifurcation feature doesn't depend on the default
Devmap mode so that "DevMapModeAuto" is unnecessary in retrieving
the PCIe Devmap default mode.

Signed-off-by: Minh Nguyen <minhnguyen1@os.amperecomputing.com>
---
 Silicon/Ampere/AmpereAltraPkg/Drivers/PcieInitPei/RootComplexNVParam.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Silicon/Ampere/AmpereAltraPkg/Drivers/PcieInitPei/RootComplexNVParam.c b/Silicon/Ampere/AmpereAltraPkg/Drivers/PcieInitPei/RootComplexNVParam.c
index a8e23015b605..199929bfdb24 100644
--- a/Silicon/Ampere/AmpereAltraPkg/Drivers/PcieInitPei/RootComplexNVParam.c
+++ b/Silicon/Ampere/AmpereAltraPkg/Drivers/PcieInitPei/RootComplexNVParam.c
@@ -160,7 +160,7 @@ GetDefaultDevMap (
   UINT8        StartIndex;
   DEV_MAP_MODE DevMapMode;
 
-  DevMapMode = MaxDevMapMode;
+  DevMapMode = DevMapMode4;
   StartIndex = IsGetDevMapLow ? PcieController0 : PcieController4;
 
   while (DevMapMode >= DevMapMode1)
-- 
2.39.0

Re: [edk2-platforms][PATCH 1/3] AmpereAltraPkg: Remove PCIe "DevMapModeAuto" case in retrieving Devmap mode

[Tinh Nguyen]

Reviewed-by: Tinh Nguyen <tinhnguyen@os.amperecomputing.com>

On 11/05/2023 14:51, Minh Nguyen wrote:
> Because PCIe Auto bifurcation feature doesn't depend on the default
> Devmap mode so that "DevMapModeAuto" is unnecessary in retrieving
> the PCIe Devmap default mode.
>
> Signed-off-by: Minh Nguyen <minhnguyen1@os.amperecomputing.com>
> ---
>   Silicon/Ampere/AmpereAltraPkg/Drivers/PcieInitPei/RootComplexNVParam.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Silicon/Ampere/AmpereAltraPkg/Drivers/PcieInitPei/RootComplexNVParam.c b/Silicon/Ampere/AmpereAltraPkg/Drivers/PcieInitPei/RootComplexNVParam.c
> index a8e23015b605..199929bfdb24 100644
> --- a/Silicon/Ampere/AmpereAltraPkg/Drivers/PcieInitPei/RootComplexNVParam.c
> +++ b/Silicon/Ampere/AmpereAltraPkg/Drivers/PcieInitPei/RootComplexNVParam.c
> @@ -160,7 +160,7 @@ GetDefaultDevMap (
>     UINT8        StartIndex;
>     DEV_MAP_MODE DevMapMode;
>   
> -  DevMapMode = MaxDevMapMode;
> +  DevMapMode = DevMapMode4;
>     StartIndex = IsGetDevMapLow ? PcieController0 : PcieController4;
>   
>     while (DevMapMode >= DevMapMode1)

[edk2-platforms][PATCH 2/3] AmpereAltraPkg: Fix PCIe link training sequence

[Minh Nguyen]

Currently, the implementation starts link training and put device
out-of-reset right away. This make LinkStat and BlockEventStat registers
indicate that link is not up yet (LinkStat: 0x300 and BlockEventStat: 0x0).
This fix will start link training after putting device out-of-reset.
The values of LinkStat and BlockEventStat registers after this fix are
respectively 0x1103 and 0x1 (Link is up).

Signed-off-by: Minh Nguyen <minhnguyen1@os.amperecomputing.com>
---
 Silicon/Ampere/AmpereAltraPkg/Library/Ac01PcieLib/PcieCore.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/Silicon/Ampere/AmpereAltraPkg/Library/Ac01PcieLib/PcieCore.c b/Silicon/Ampere/AmpereAltraPkg/Library/Ac01PcieLib/PcieCore.c
index f7c8defc1906..d797442d9fbc 100644
--- a/Silicon/Ampere/AmpereAltraPkg/Library/Ac01PcieLib/PcieCore.c
+++ b/Silicon/Ampere/AmpereAltraPkg/Library/Ac01PcieLib/PcieCore.c
@@ -1177,12 +1177,12 @@ Ac01PcieCoreSetupRC (
     // Assert PERST low to reset endpoint
     BoardPcieAssertPerst (RootComplex, PcieIndex, FALSE);
 
+    // Complete the PERST pulse
+    BoardPcieAssertPerst (RootComplex, PcieIndex, TRUE);
+
     // Start link training
     StartLinkTraining (RootComplex, PcieIndex, TRUE);
 
-    // Complete the PERST pulse
-    BoardPcieAssertPerst (RootComplex, PcieIndex, TRUE);
-
     // Lock programming of config space
     EnableDbiAccess  (RootComplex, PcieIndex, FALSE);
 
-- 
2.39.0

Re: [edk2-platforms][PATCH 2/3] AmpereAltraPkg: Fix PCIe link training sequence

[Tinh Nguyen]

Reviewed-by: Tinh Nguyen <tinhnguyen@os.amperecomputing.com>


@Leif, @Ard, Please help us to merge this series

Thanks,

- Tinh

On 11/05/2023 14:51, Minh Nguyen wrote:
> Currently, the implementation starts link training and put device
> out-of-reset right away. This make LinkStat and BlockEventStat registers
> indicate that link is not up yet (LinkStat: 0x300 and BlockEventStat: 0x0).
> This fix will start link training after putting device out-of-reset.
> The values of LinkStat and BlockEventStat registers after this fix are
> respectively 0x1103 and 0x1 (Link is up).
>
> Signed-off-by: Minh Nguyen <minhnguyen1@os.amperecomputing.com>
> ---
>   Silicon/Ampere/AmpereAltraPkg/Library/Ac01PcieLib/PcieCore.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/Silicon/Ampere/AmpereAltraPkg/Library/Ac01PcieLib/PcieCore.c b/Silicon/Ampere/AmpereAltraPkg/Library/Ac01PcieLib/PcieCore.c
> index f7c8defc1906..d797442d9fbc 100644
> --- a/Silicon/Ampere/AmpereAltraPkg/Library/Ac01PcieLib/PcieCore.c
> +++ b/Silicon/Ampere/AmpereAltraPkg/Library/Ac01PcieLib/PcieCore.c
> @@ -1177,12 +1177,12 @@ Ac01PcieCoreSetupRC (
>       // Assert PERST low to reset endpoint
>       BoardPcieAssertPerst (RootComplex, PcieIndex, FALSE);
>   
> +    // Complete the PERST pulse
> +    BoardPcieAssertPerst (RootComplex, PcieIndex, TRUE);
> +
>       // Start link training
>       StartLinkTraining (RootComplex, PcieIndex, TRUE);
>   
> -    // Complete the PERST pulse
> -    BoardPcieAssertPerst (RootComplex, PcieIndex, TRUE);
> -
>       // Lock programming of config space
>       EnableDbiAccess  (RootComplex, PcieIndex, FALSE);
>   

[edk2-platforms][PATCH 3/3] AmpereAltraPkg: Add retry logic when PCIe card is present

[Minh Nguyen]

From: Tinh Nguyen <tinhnguyen@os.amperecomputing.com>

Add PCIe link retry logic. If the card is detected to be present,
allow a retry for 3 times.

When the link is down, check if the LTSSMEN bit is cleared.
Based on Altra PCIe IP, the LTSSMEN bit is cleared from 1 to 0
if there is a surprising link down.
This indicates that a card is present.

Signed-off-by: Minh Nguyen <minhnguyen1@os.amperecomputing.com>
---
 Silicon/Ampere/AmpereAltraPkg/Library/Ac01PcieLib/PcieCore.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/Silicon/Ampere/AmpereAltraPkg/Library/Ac01PcieLib/PcieCore.c b/Silicon/Ampere/AmpereAltraPkg/Library/Ac01PcieLib/PcieCore.c
index d797442d9fbc..9cc059788581 100644
--- a/Silicon/Ampere/AmpereAltraPkg/Library/Ac01PcieLib/PcieCore.c
+++ b/Silicon/Ampere/AmpereAltraPkg/Library/Ac01PcieLib/PcieCore.c
@@ -1769,7 +1769,11 @@ Ac01PcieCoreUpdateLink (
       } else {
         FailedPciePtr[*FailedPcieCount] = PcieIndex;
         *FailedPcieCount += 1;
-        *IsNextRoundNeeded = !(*IsNextRoundNeeded) ? Ac01PcieCoreCheckCardPresent (Pcie) : TRUE;
+
+        if (Ac01PcieCoreCheckCardPresent (Pcie)) {
+          *IsNextRoundNeeded = TRUE;
+          DEBUG ((DEBUG_INFO, "PCIE%d.%d Link retry\n", RootComplex->ID, PcieIndex));
+        }
       }
     }
   }
-- 
2.39.0

Re: [edk2-platforms][PATCH 0/3] Add necessary changes for PCIe core and remove unnecessary checks for PCIe Devmap mode

[Nhi Pham]

Hi Minh,

Thanks for this patchset. It looks good to me.

Reviewed-by: Nhi Pham <nhi@os.amperecomputing.com>

Hi Leif, Ard - Could you please help merge this series?

Thanks,

Nhi

On 5/11/2023 2:51 PM, Minh Nguyen wrote:
> These patches help to improve PCIe core for Link training sequence and
> add logic to check PCIe card is present or not. One patch also removes
> unnecessary checks for PCIe Devmap mode.
>
> Minh Nguyen (2):
>    AmpereAltraPkg: Remove PCIe "DevMapModeAuto" case in retrieving Devmap
>      mode
>    AmpereAltraPkg: Fix PCIe link training sequence
>
> Tinh Nguyen (1):
>    AmpereAltraPkg: Add retry logic when PCIe card is present
>
>   Silicon/Ampere/AmpereAltraPkg/Drivers/PcieInitPei/RootComplexNVParam.c |  2 +-
>   Silicon/Ampere/AmpereAltraPkg/Library/Ac01PcieLib/PcieCore.c           | 12 ++++++++----
>   2 files changed, 9 insertions(+), 5 deletions(-)
>

Re: [edk2-platforms][PATCH 0/3] Add necessary changes for PCIe core and remove unnecessary checks for PCIe Devmap mode

[Minh Nguyen]

Hi Leif and Ard

  Could you please help merge this series?

Thanks,

Minh Nguyen

On 5/11/2023 2:51 PM, Minh Nguyen wrote:
> These patches help to improve PCIe core for Link training sequence and
> add logic to check PCIe card is present or not. One patch also removes
> unnecessary checks for PCIe Devmap mode.
>
> Minh Nguyen (2):
>    AmpereAltraPkg: Remove PCIe "DevMapModeAuto" case in retrieving Devmap
>      mode
>    AmpereAltraPkg: Fix PCIe link training sequence
>
> Tinh Nguyen (1):
>    AmpereAltraPkg: Add retry logic when PCIe card is present
>
>   Silicon/Ampere/AmpereAltraPkg/Drivers/PcieInitPei/RootComplexNVParam.c |  2 +-
>   Silicon/Ampere/AmpereAltraPkg/Library/Ac01PcieLib/PcieCore.c           | 12 ++++++++----
>   2 files changed, 9 insertions(+), 5 deletions(-)
>

Re: [edk2-platforms][PATCH 0/3] Add necessary changes for PCIe core and remove unnecessary checks for PCIe Devmap mode

[Ard Biesheuvel]

On Wed, 24 May 2023 at 02:45, Minh Nguyen
<minhnguyen1@amperemail.onmicrosoft.com> wrote:
>
> Hi Leif and Ard
>
>   Could you please help merge this series?
>

Pushed as 4303f7782c08..a869bae89a6d

Thanks,

>
> On 5/11/2023 2:51 PM, Minh Nguyen wrote:
> > These patches help to improve PCIe core for Link training sequence and
> > add logic to check PCIe card is present or not. One patch also removes
> > unnecessary checks for PCIe Devmap mode.
> >
> > Minh Nguyen (2):
> >    AmpereAltraPkg: Remove PCIe "DevMapModeAuto" case in retrieving Devmap
> >      mode
> >    AmpereAltraPkg: Fix PCIe link training sequence
> >
> > Tinh Nguyen (1):
> >    AmpereAltraPkg: Add retry logic when PCIe card is present
> >
> >   Silicon/Ampere/AmpereAltraPkg/Drivers/PcieInitPei/RootComplexNVParam.c |  2 +-
> >   Silicon/Ampere/AmpereAltraPkg/Library/Ac01PcieLib/PcieCore.c           | 12 ++++++++----
> >   2 files changed, 9 insertions(+), 5 deletions(-)
> >