* [PATCH] MdeModulePkg/SdMmcPciHcDxe: Add V3 64b DMA Support
@ 2019-03-01 18:30 Ashish Singhal
2019-03-02 16:00 ` Gao, Liming
2019-03-06 3:01 ` Wu, Hao A
0 siblings, 2 replies; 9+ messages in thread
From: Ashish Singhal @ 2019-03-01 18:30 UTC (permalink / raw)
To: edk2-devel; +Cc: hao.a.wu, eugene, Ashish Singhal
Driver was supporting only 32b DMA support for V3 controllers. Add
support for 64b DMA as well for completeness.
For V4.0 64b support, driver was looking at incorrect capability
register bit. Fix for that is present as well.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ashish Singhal <ashishsingha@nvidia.com>
---
MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c | 10 +-
MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h | 6 +-
MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c | 199 ++++++++++++++-------
MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h | 29 ++-
4 files changed, 170 insertions(+), 74 deletions(-)
diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
index b474f8d..9b7b88c 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
@@ -6,7 +6,7 @@
It would expose EFI_SD_MMC_PASS_THRU_PROTOCOL for upper layer use.
- Copyright (c) 2018, NVIDIA CORPORATION. All rights reserved.
+ Copyright (c) 2018-2019, NVIDIA CORPORATION. All rights reserved.
Copyright (c) 2015 - 2019, Intel Corporation. All rights reserved.<BR>
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD License
@@ -666,8 +666,12 @@ SdMmcPciHcDriverBindingStart (
// If any of the slots does not support 64b system bus
// do not enable 64b DMA in the PCI layer.
//
- if (Private->Capability[Slot].SysBus64V3 == 0 &&
- Private->Capability[Slot].SysBus64V4 == 0) {
+ if ((Private->ControllerVersion[Slot] == SD_MMC_HC_CTRL_VER_300 &&
+ Private->Capability[Slot].SysBus64V3 == 0) ||
+ (Private->ControllerVersion[Slot] == SD_MMC_HC_CTRL_VER_400 &&
+ Private->Capability[Slot].SysBus64V3 == 0) ||
+ (Private->ControllerVersion[Slot] >= SD_MMC_HC_CTRL_VER_410 &&
+ Private->Capability[Slot].SysBus64V4 == 0)) {
Support64BitDma = FALSE;
}
diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
index 1bb701a..68d8a5c 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
@@ -2,7 +2,7 @@
Provides some data structure definitions used by the SD/MMC host controller driver.
-Copyright (c) 2018, NVIDIA CORPORATION. All rights reserved.
+Copyright (c) 2018-2019, NVIDIA CORPORATION. All rights reserved.
Copyright (c) 2015, Intel Corporation. All rights reserved.<BR>
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD License
@@ -145,13 +145,15 @@ typedef struct {
EFI_PHYSICAL_ADDRESS DataPhy;
VOID *DataMap;
SD_MMC_HC_TRANSFER_MODE Mode;
+ SD_MMC_HC_ADMA_LEGTH Length;
EFI_EVENT Event;
BOOLEAN Started;
UINT64 Timeout;
SD_MMC_HC_ADMA_32_DESC_LINE *Adma32Desc;
- SD_MMC_HC_ADMA_64_DESC_LINE *Adma64Desc;
+ SD_MMC_HC_ADMA_64_V3_DESC_LINE *Adma64V3Desc;
+ SD_MMC_HC_ADMA_64_V4_DESC_LINE *Adma64V4Desc;
EFI_PHYSICAL_ADDRESS AdmaDescPhy;
VOID *AdmaMap;
UINT32 AdmaPages;
diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
index d73fa10..a6d2395 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
@@ -6,7 +6,7 @@
It would expose EFI_SD_MMC_PASS_THRU_PROTOCOL for upper layer use.
- Copyright (c) 2018, NVIDIA CORPORATION. All rights reserved.
+ Copyright (c) 2018-2019, NVIDIA CORPORATION. All rights reserved.
Copyright (c) 2015 - 2019, Intel Corporation. All rights reserved.<BR>
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD License
@@ -1010,18 +1010,32 @@ SdMmcHcInitV4Enhancements (
if (ControllerVer >= SD_MMC_HC_CTRL_VER_400) {
HostCtrl2 = SD_MMC_HC_V4_EN;
//
- // Check if V4 64bit support is available
+ // Check if controller version V4.0
//
- if (Capability.SysBus64V4 != 0) {
- HostCtrl2 |= SD_MMC_HC_64_ADDR_EN;
- DEBUG ((DEBUG_INFO, "Enabled V4 64 bit system bus support\n"));
+ if (ControllerVer == SD_MMC_HC_CTRL_VER_400) {
+ //
+ // Check if 64bit support is available
+ //
+ if (Capability.SysBus64V3 != 0) {
+ HostCtrl2 |= SD_MMC_HC_64_ADDR_EN;
+ DEBUG ((DEBUG_INFO, "Enabled V4 64 bit system bus support\n"));
+ }
}
//
// Check if controller version V4.10 or higher
//
- if (ControllerVer >= SD_MMC_HC_CTRL_VER_410) {
- HostCtrl2 |= SD_MMC_HC_26_DATA_LEN_ADMA_EN;
- DEBUG ((DEBUG_INFO, "Enabled V4 26 bit data length ADMA support\n"));
+ else if (ControllerVer >= SD_MMC_HC_CTRL_VER_410) {
+ //
+ // Check if 64bit support is available
+ //
+ if (Capability.SysBus64V4 != 0) {
+ HostCtrl2 |= SD_MMC_HC_64_ADDR_EN;
+ DEBUG ((DEBUG_INFO, "Enabled V4 64 bit system bus support\n"));
+ }
+ if (ControllerVer >= SD_MMC_HC_CTRL_VER_410) {
+ HostCtrl2 |= SD_MMC_HC_26_DATA_LEN_ADMA_EN;
+ DEBUG ((DEBUG_INFO, "Enabled V4 26 bit data length ADMA support\n"));
+ }
}
Status = SdMmcHcOrMmio (PciIo, Slot, SD_MMC_HC_HOST_CTRL2, sizeof (HostCtrl2), &HostCtrl2);
if (EFI_ERROR (Status)) {
@@ -1393,14 +1407,10 @@ BuildAdmaDescTable (
EFI_PCI_IO_PROTOCOL *PciIo;
EFI_STATUS Status;
UINTN Bytes;
- BOOLEAN AddressingMode64;
- BOOLEAN DataLength26;
UINT32 AdmaMaxDataPerLine;
UINT32 DescSize;
VOID *AdmaDesc;
- AddressingMode64 = FALSE;
- DataLength26 = FALSE;
AdmaMaxDataPerLine = ADMA_MAX_DATA_PER_LINE_16B;
DescSize = sizeof (SD_MMC_HC_ADMA_32_DESC_LINE);
AdmaDesc = NULL;
@@ -1410,27 +1420,16 @@ BuildAdmaDescTable (
PciIo = Trb->Private->PciIo;
//
- // Detect whether 64bit addressing is supported.
- //
- if (ControllerVer >= SD_MMC_HC_CTRL_VER_400) {
- Status = SdMmcHcCheckMmioSet(PciIo, Trb->Slot, SD_MMC_HC_HOST_CTRL2, sizeof(UINT16),
- SD_MMC_HC_V4_EN|SD_MMC_HC_64_ADDR_EN, SD_MMC_HC_V4_EN|SD_MMC_HC_64_ADDR_EN);
- if (!EFI_ERROR (Status)) {
- AddressingMode64 = TRUE;
- DescSize = sizeof (SD_MMC_HC_ADMA_64_DESC_LINE);
- }
- }
- //
// Check for valid ranges in 32bit ADMA Descriptor Table
//
- if (!AddressingMode64 &&
+ if ((Trb->Mode == SdMmcAdma32bMode) &&
((Data >= 0x100000000ul) || ((Data + DataLen) > 0x100000000ul))) {
return EFI_INVALID_PARAMETER;
}
//
// Check address field alignment
//
- if (AddressingMode64) {
+ if (Trb->Mode != SdMmcAdma32bMode) {
//
// Address field shall be set on 64-bit boundary (Lower 3-bit is always set to 0)
//
@@ -1445,13 +1444,19 @@ BuildAdmaDescTable (
DEBUG ((DEBUG_INFO, "The buffer [0x%x] to construct ADMA desc is not aligned to 4 bytes boundary!\n", Data));
}
}
+
//
- // Detect whether 26bit data length is supported.
+ // Configure 64b ADMA.
//
- Status = SdMmcHcCheckMmioSet(PciIo, Trb->Slot, SD_MMC_HC_HOST_CTRL2, sizeof(UINT16),
- SD_MMC_HC_26_DATA_LEN_ADMA_EN, SD_MMC_HC_26_DATA_LEN_ADMA_EN);
- if (!EFI_ERROR (Status)) {
- DataLength26 = TRUE;
+ if (Trb->Mode == SdMmcAdma64bV3Mode) {
+ DescSize = sizeof (SD_MMC_HC_ADMA_64_V3_DESC_LINE);
+ }else if (Trb->Mode == SdMmcAdma64bV4Mode) {
+ DescSize = sizeof (SD_MMC_HC_ADMA_64_V4_DESC_LINE);
+ }
+ //
+ // Configure 26b data length.
+ //
+ if (Trb->Length == SdMmcAdmaLen26b) {
AdmaMaxDataPerLine = ADMA_MAX_DATA_PER_LINE_26B;
}
@@ -1492,7 +1497,7 @@ BuildAdmaDescTable (
return EFI_OUT_OF_RESOURCES;
}
- if ((!AddressingMode64) &&
+ if ((Trb->Mode == SdMmcAdma32bMode) &&
(UINT64)(UINTN)Trb->AdmaDescPhy > 0x100000000ul) {
//
// The ADMA doesn't support 64bit addressing.
@@ -1511,19 +1516,26 @@ BuildAdmaDescTable (
Remaining = DataLen;
Address = Data;
- if (!AddressingMode64) {
+ if (Trb->Mode == SdMmcAdma32bMode) {
Trb->Adma32Desc = AdmaDesc;
- Trb->Adma64Desc = NULL;
+ Trb->Adma64V3Desc = NULL;
+ Trb->Adma64V4Desc = NULL;
+ } else if (Trb->Mode == SdMmcAdma64bV3Mode) {
+ Trb->Adma64V3Desc = AdmaDesc;
+ Trb->Adma32Desc = NULL;
+ Trb->Adma64V4Desc = NULL;
} else {
- Trb->Adma64Desc = AdmaDesc;
+ Trb->Adma64V4Desc = AdmaDesc;
Trb->Adma32Desc = NULL;
+ Trb->Adma64V3Desc = NULL;
}
+
for (Index = 0; Index < Entries; Index++) {
- if (!AddressingMode64) {
+ if (Trb->Mode == SdMmcAdma32bMode) {
if (Remaining <= AdmaMaxDataPerLine) {
Trb->Adma32Desc[Index].Valid = 1;
Trb->Adma32Desc[Index].Act = 2;
- if (DataLength26) {
+ if (Trb->Length == SdMmcAdmaLen26b) {
Trb->Adma32Desc[Index].UpperLength = (UINT16)RShiftU64 (Remaining, 16);
}
Trb->Adma32Desc[Index].LowerLength = (UINT16)(Remaining & MAX_UINT16);
@@ -1532,32 +1544,53 @@ BuildAdmaDescTable (
} else {
Trb->Adma32Desc[Index].Valid = 1;
Trb->Adma32Desc[Index].Act = 2;
- if (DataLength26) {
+ if (Trb->Length == SdMmcAdmaLen26b) {
Trb->Adma32Desc[Index].UpperLength = 0;
}
Trb->Adma32Desc[Index].LowerLength = 0;
Trb->Adma32Desc[Index].Address = (UINT32)Address;
}
+ } else if (Trb->Mode == SdMmcAdma64bV3Mode) {
+ if (Remaining <= AdmaMaxDataPerLine) {
+ Trb->Adma64V3Desc[Index].Valid = 1;
+ Trb->Adma64V3Desc[Index].Act = 2;
+ if (Trb->Length == SdMmcAdmaLen26b) {
+ Trb->Adma64V3Desc[Index].UpperLength = (UINT16)RShiftU64 (Remaining, 16);
+ }
+ Trb->Adma64V3Desc[Index].LowerLength = (UINT16)(Remaining & MAX_UINT16);
+ Trb->Adma64V3Desc[Index].LowerAddress = (UINT32)Address;
+ Trb->Adma64V3Desc[Index].UpperAddress = (UINT32)RShiftU64 (Address, 32);
+ break;
+ } else {
+ Trb->Adma64V3Desc[Index].Valid = 1;
+ Trb->Adma64V3Desc[Index].Act = 2;
+ if (Trb->Length == SdMmcAdmaLen26b) {
+ Trb->Adma64V3Desc[Index].UpperLength = 0;
+ }
+ Trb->Adma64V3Desc[Index].LowerLength = 0;
+ Trb->Adma64V3Desc[Index].LowerAddress = (UINT32)Address;
+ Trb->Adma64V3Desc[Index].UpperAddress = (UINT32)RShiftU64 (Address, 32);
+ }
} else {
if (Remaining <= AdmaMaxDataPerLine) {
- Trb->Adma64Desc[Index].Valid = 1;
- Trb->Adma64Desc[Index].Act = 2;
- if (DataLength26) {
- Trb->Adma64Desc[Index].UpperLength = (UINT16)RShiftU64 (Remaining, 16);
+ Trb->Adma64V4Desc[Index].Valid = 1;
+ Trb->Adma64V4Desc[Index].Act = 2;
+ if (Trb->Length == SdMmcAdmaLen26b) {
+ Trb->Adma64V4Desc[Index].UpperLength = (UINT16)RShiftU64 (Remaining, 16);
}
- Trb->Adma64Desc[Index].LowerLength = (UINT16)(Remaining & MAX_UINT16);
- Trb->Adma64Desc[Index].LowerAddress = (UINT32)Address;
- Trb->Adma64Desc[Index].UpperAddress = (UINT32)RShiftU64 (Address, 32);
+ Trb->Adma64V4Desc[Index].LowerLength = (UINT16)(Remaining & MAX_UINT16);
+ Trb->Adma64V4Desc[Index].LowerAddress = (UINT32)Address;
+ Trb->Adma64V4Desc[Index].UpperAddress = (UINT32)RShiftU64 (Address, 32);
break;
} else {
- Trb->Adma64Desc[Index].Valid = 1;
- Trb->Adma64Desc[Index].Act = 2;
- if (DataLength26) {
- Trb->Adma64Desc[Index].UpperLength = 0;
+ Trb->Adma64V4Desc[Index].Valid = 1;
+ Trb->Adma64V4Desc[Index].Act = 2;
+ if (Trb->Length == SdMmcAdmaLen26b) {
+ Trb->Adma64V4Desc[Index].UpperLength = 0;
}
- Trb->Adma64Desc[Index].LowerLength = 0;
- Trb->Adma64Desc[Index].LowerAddress = (UINT32)Address;
- Trb->Adma64Desc[Index].UpperAddress = (UINT32)RShiftU64 (Address, 32);
+ Trb->Adma64V4Desc[Index].LowerLength = 0;
+ Trb->Adma64V4Desc[Index].LowerAddress = (UINT32)Address;
+ Trb->Adma64V4Desc[Index].UpperAddress = (UINT32)RShiftU64 (Address, 32);
}
}
@@ -1568,7 +1601,13 @@ BuildAdmaDescTable (
//
// Set the last descriptor line as end of descriptor table
//
- AddressingMode64 ? (Trb->Adma64Desc[Index].End = 1) : (Trb->Adma32Desc[Index].End = 1);
+ if (Trb->Mode == SdMmcAdma32bMode) {
+ Trb->Adma32Desc[Index].End = 1;
+ } else if (Trb->Mode == SdMmcAdma64bV3Mode) {
+ Trb->Adma64V3Desc[Index].End = 1;
+ } else {
+ Trb->Adma64V4Desc[Index].End = 1;
+ }
return EFI_SUCCESS;
}
@@ -1665,7 +1704,20 @@ SdMmcCreateTrb (
if (Trb->DataLen == 0) {
Trb->Mode = SdMmcNoData;
} else if (Private->Capability[Slot].Adma2 != 0) {
- Trb->Mode = SdMmcAdmaMode;
+ Trb->Mode = SdMmcAdma32bMode;
+ Trb->Length = SdMmcAdmaLen16b;
+ if ((Private->ControllerVersion[Slot] == SD_MMC_HC_CTRL_VER_300) &&
+ (Private->Capability[Slot].SysBus64V3 == 1)) {
+ Trb->Mode = SdMmcAdma64bV3Mode;
+ } else if (((Private->ControllerVersion[Slot] == SD_MMC_HC_CTRL_VER_400) &&
+ (Private->Capability[Slot].SysBus64V3 == 1)) ||
+ ((Private->ControllerVersion[Slot] >= SD_MMC_HC_CTRL_VER_410) &&
+ (Private->Capability[Slot].SysBus64V4 == 1))) {
+ Trb->Mode = SdMmcAdma64bV4Mode;
+ }
+ if (Private->ControllerVersion[Slot] >= SD_MMC_HC_CTRL_VER_410) {
+ Trb->Length = SdMmcAdmaLen26b;
+ }
Status = BuildAdmaDescTable (Trb, Private->ControllerVersion[Slot]);
if (EFI_ERROR (Status)) {
PciIo->Unmap (PciIo, Trb->DataMap);
@@ -1719,11 +1771,18 @@ SdMmcFreeTrb (
Trb->Adma32Desc
);
}
- if (Trb->Adma64Desc != NULL) {
+ if (Trb->Adma64V3Desc != NULL) {
+ PciIo->FreeBuffer (
+ PciIo,
+ Trb->AdmaPages,
+ Trb->Adma64V3Desc
+ );
+ }
+ if (Trb->Adma64V4Desc != NULL) {
PciIo->FreeBuffer (
PciIo,
Trb->AdmaPages,
- Trb->Adma64Desc
+ Trb->Adma64V4Desc
);
}
if (Trb->DataMap != NULL) {
@@ -1891,27 +1950,35 @@ SdMmcExecTrb (
if (EFI_ERROR (Status)) {
return Status;
}
+
+ if (Private->ControllerVersion[Trb->Slot] >= SD_MMC_HC_CTRL_VER_400) {
+ Status = SdMmcHcCheckMmioSet(PciIo, Trb->Slot, SD_MMC_HC_HOST_CTRL2, sizeof(UINT16),
+ SD_MMC_HC_64_ADDR_EN, SD_MMC_HC_64_ADDR_EN);
+ if (!EFI_ERROR (Status)) {
+ AddressingMode64 = TRUE;
+ }
+ }
+
//
// Set Host Control 1 register DMA Select field
//
- if (Trb->Mode == SdMmcAdmaMode) {
+ if ((Trb->Mode == SdMmcAdma32bMode) ||
+ (Trb->Mode == SdMmcAdma64bV4Mode)) {
HostCtrl1 = BIT4;
Status = SdMmcHcOrMmio (PciIo, Trb->Slot, SD_MMC_HC_HOST_CTRL1, sizeof (HostCtrl1), &HostCtrl1);
if (EFI_ERROR (Status)) {
return Status;
}
+ } else if (Trb->Mode == SdMmcAdma64bV3Mode) {
+ HostCtrl1 = BIT4|BIT3;
+ Status = SdMmcHcOrMmio (PciIo, Trb->Slot, SD_MMC_HC_HOST_CTRL1, sizeof (HostCtrl1), &HostCtrl1);
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
}
SdMmcHcLedOnOff (PciIo, Trb->Slot, TRUE);
- if (Private->ControllerVersion[Trb->Slot] >= SD_MMC_HC_CTRL_VER_400) {
- Status = SdMmcHcCheckMmioSet(PciIo, Trb->Slot, SD_MMC_HC_HOST_CTRL2, sizeof(UINT16),
- SD_MMC_HC_V4_EN|SD_MMC_HC_64_ADDR_EN, SD_MMC_HC_V4_EN|SD_MMC_HC_64_ADDR_EN);
- if (!EFI_ERROR (Status)) {
- AddressingMode64 = TRUE;
- }
- }
-
if (Trb->Mode == SdMmcSdmaMode) {
if ((!AddressingMode64) &&
((UINT64)(UINTN)Trb->DataPhy >= 0x100000000ul)) {
@@ -1929,7 +1996,9 @@ SdMmcExecTrb (
if (EFI_ERROR (Status)) {
return Status;
}
- } else if (Trb->Mode == SdMmcAdmaMode) {
+ } else if ((Trb->Mode == SdMmcAdma32bMode) ||
+ (Trb->Mode == SdMmcAdma64bV3Mode) ||
+ (Trb->Mode == SdMmcAdma64bV4Mode)) {
AdmaAddr = (UINT64)(UINTN)Trb->AdmaDescPhy;
Status = SdMmcHcRwMmio (PciIo, Trb->Slot, SD_MMC_HC_ADMA_SYS_ADDR, FALSE, sizeof (AdmaAddr), &AdmaAddr);
if (EFI_ERROR (Status)) {
diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
index d157f2c..3a05456 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
@@ -2,7 +2,7 @@
Provides some data structure definitions used by the SD/MMC host controller driver.
-Copyright (c) 2018, NVIDIA CORPORATION. All rights reserved.
+Copyright (c) 2018-2019, NVIDIA CORPORATION. All rights reserved.
Copyright (c) 2015, Intel Corporation. All rights reserved.<BR>
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD License
@@ -80,16 +80,25 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
//
// The transfer modes supported by SD Host Controller
-// Simplified Spec 3.0 Table 1-2
//
typedef enum {
SdMmcNoData,
SdMmcPioMode,
SdMmcSdmaMode,
- SdMmcAdmaMode
+ SdMmcAdma32bMode,
+ SdMmcAdma64bV3Mode,
+ SdMmcAdma64bV4Mode
} SD_MMC_HC_TRANSFER_MODE;
//
+// The ADMA transfer lengths supported by SD Host Controller
+//
+typedef enum {
+ SdMmcAdmaLen16b,
+ SdMmcAdmaLen26b
+} SD_MMC_HC_ADMA_LEGTH;
+
+//
// The maximum data length of each descriptor line
//
#define ADMA_MAX_DATA_PER_LINE_16B SIZE_64KB
@@ -122,8 +131,20 @@ typedef struct {
UINT32 LowerLength:16;
UINT32 LowerAddress;
UINT32 UpperAddress;
+} SD_MMC_HC_ADMA_64_V3_DESC_LINE;
+
+typedef struct {
+ UINT32 Valid:1;
+ UINT32 End:1;
+ UINT32 Int:1;
+ UINT32 Reserved:1;
+ UINT32 Act:2;
+ UINT32 UpperLength:10;
+ UINT32 LowerLength:16;
+ UINT32 LowerAddress;
+ UINT32 UpperAddress;
UINT32 Reserved1;
-} SD_MMC_HC_ADMA_64_DESC_LINE;
+} SD_MMC_HC_ADMA_64_V4_DESC_LINE;
#define SD_MMC_SDMA_BOUNDARY 512 * 1024
#define SD_MMC_SDMA_ROUND_UP(x, n) (((x) + n) & ~(n - 1))
--
2.7.4
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] MdeModulePkg/SdMmcPciHcDxe: Add V3 64b DMA Support
2019-03-01 18:30 [PATCH] MdeModulePkg/SdMmcPciHcDxe: Add V3 64b DMA Support Ashish Singhal
@ 2019-03-02 16:00 ` Gao, Liming
2019-03-04 4:08 ` Ashish Singhal
2019-03-06 3:01 ` Wu, Hao A
1 sibling, 1 reply; 9+ messages in thread
From: Gao, Liming @ 2019-03-02 16:00 UTC (permalink / raw)
To: Ashish Singhal, edk2-devel@lists.01.org; +Cc: Wu, Hao A
Ashish:
This seems a feature. Now, we are in Hard Feature Freeze. So, it will not be added until edk2-stable201903 tag is created at 2019-03-08.
Thanks
Liming
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ashish Singhal
> Sent: Saturday, March 2, 2019 2:30 AM
> To: edk2-devel@lists.01.org
> Cc: Wu, Hao A <hao.a.wu@intel.com>; Ashish Singhal <ashishsingha@nvidia.com>
> Subject: [edk2] [PATCH] MdeModulePkg/SdMmcPciHcDxe: Add V3 64b DMA Support
>
> Driver was supporting only 32b DMA support for V3 controllers. Add
> support for 64b DMA as well for completeness.
>
> For V4.0 64b support, driver was looking at incorrect capability
> register bit. Fix for that is present as well.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ashish Singhal <ashishsingha@nvidia.com>
> ---
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c | 10 +-
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h | 6 +-
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c | 199 ++++++++++++++-------
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h | 29 ++-
> 4 files changed, 170 insertions(+), 74 deletions(-)
>
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> index b474f8d..9b7b88c 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> @@ -6,7 +6,7 @@
>
> It would expose EFI_SD_MMC_PASS_THRU_PROTOCOL for upper layer use.
>
> - Copyright (c) 2018, NVIDIA CORPORATION. All rights reserved.
> + Copyright (c) 2018-2019, NVIDIA CORPORATION. All rights reserved.
> Copyright (c) 2015 - 2019, Intel Corporation. All rights reserved.<BR>
> This program and the accompanying materials
> are licensed and made available under the terms and conditions of the BSD License
> @@ -666,8 +666,12 @@ SdMmcPciHcDriverBindingStart (
> // If any of the slots does not support 64b system bus
> // do not enable 64b DMA in the PCI layer.
> //
> - if (Private->Capability[Slot].SysBus64V3 == 0 &&
> - Private->Capability[Slot].SysBus64V4 == 0) {
> + if ((Private->ControllerVersion[Slot] == SD_MMC_HC_CTRL_VER_300 &&
> + Private->Capability[Slot].SysBus64V3 == 0) ||
> + (Private->ControllerVersion[Slot] == SD_MMC_HC_CTRL_VER_400 &&
> + Private->Capability[Slot].SysBus64V3 == 0) ||
> + (Private->ControllerVersion[Slot] >= SD_MMC_HC_CTRL_VER_410 &&
> + Private->Capability[Slot].SysBus64V4 == 0)) {
> Support64BitDma = FALSE;
> }
>
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> index 1bb701a..68d8a5c 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> @@ -2,7 +2,7 @@
>
> Provides some data structure definitions used by the SD/MMC host controller driver.
>
> -Copyright (c) 2018, NVIDIA CORPORATION. All rights reserved.
> +Copyright (c) 2018-2019, NVIDIA CORPORATION. All rights reserved.
> Copyright (c) 2015, Intel Corporation. All rights reserved.<BR>
> This program and the accompanying materials
> are licensed and made available under the terms and conditions of the BSD License
> @@ -145,13 +145,15 @@ typedef struct {
> EFI_PHYSICAL_ADDRESS DataPhy;
> VOID *DataMap;
> SD_MMC_HC_TRANSFER_MODE Mode;
> + SD_MMC_HC_ADMA_LEGTH Length;
>
> EFI_EVENT Event;
> BOOLEAN Started;
> UINT64 Timeout;
>
> SD_MMC_HC_ADMA_32_DESC_LINE *Adma32Desc;
> - SD_MMC_HC_ADMA_64_DESC_LINE *Adma64Desc;
> + SD_MMC_HC_ADMA_64_V3_DESC_LINE *Adma64V3Desc;
> + SD_MMC_HC_ADMA_64_V4_DESC_LINE *Adma64V4Desc;
> EFI_PHYSICAL_ADDRESS AdmaDescPhy;
> VOID *AdmaMap;
> UINT32 AdmaPages;
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> index d73fa10..a6d2395 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> @@ -6,7 +6,7 @@
>
> It would expose EFI_SD_MMC_PASS_THRU_PROTOCOL for upper layer use.
>
> - Copyright (c) 2018, NVIDIA CORPORATION. All rights reserved.
> + Copyright (c) 2018-2019, NVIDIA CORPORATION. All rights reserved.
> Copyright (c) 2015 - 2019, Intel Corporation. All rights reserved.<BR>
> This program and the accompanying materials
> are licensed and made available under the terms and conditions of the BSD License
> @@ -1010,18 +1010,32 @@ SdMmcHcInitV4Enhancements (
> if (ControllerVer >= SD_MMC_HC_CTRL_VER_400) {
> HostCtrl2 = SD_MMC_HC_V4_EN;
> //
> - // Check if V4 64bit support is available
> + // Check if controller version V4.0
> //
> - if (Capability.SysBus64V4 != 0) {
> - HostCtrl2 |= SD_MMC_HC_64_ADDR_EN;
> - DEBUG ((DEBUG_INFO, "Enabled V4 64 bit system bus support\n"));
> + if (ControllerVer == SD_MMC_HC_CTRL_VER_400) {
> + //
> + // Check if 64bit support is available
> + //
> + if (Capability.SysBus64V3 != 0) {
> + HostCtrl2 |= SD_MMC_HC_64_ADDR_EN;
> + DEBUG ((DEBUG_INFO, "Enabled V4 64 bit system bus support\n"));
> + }
> }
> //
> // Check if controller version V4.10 or higher
> //
> - if (ControllerVer >= SD_MMC_HC_CTRL_VER_410) {
> - HostCtrl2 |= SD_MMC_HC_26_DATA_LEN_ADMA_EN;
> - DEBUG ((DEBUG_INFO, "Enabled V4 26 bit data length ADMA support\n"));
> + else if (ControllerVer >= SD_MMC_HC_CTRL_VER_410) {
> + //
> + // Check if 64bit support is available
> + //
> + if (Capability.SysBus64V4 != 0) {
> + HostCtrl2 |= SD_MMC_HC_64_ADDR_EN;
> + DEBUG ((DEBUG_INFO, "Enabled V4 64 bit system bus support\n"));
> + }
> + if (ControllerVer >= SD_MMC_HC_CTRL_VER_410) {
> + HostCtrl2 |= SD_MMC_HC_26_DATA_LEN_ADMA_EN;
> + DEBUG ((DEBUG_INFO, "Enabled V4 26 bit data length ADMA support\n"));
> + }
> }
> Status = SdMmcHcOrMmio (PciIo, Slot, SD_MMC_HC_HOST_CTRL2, sizeof (HostCtrl2), &HostCtrl2);
> if (EFI_ERROR (Status)) {
> @@ -1393,14 +1407,10 @@ BuildAdmaDescTable (
> EFI_PCI_IO_PROTOCOL *PciIo;
> EFI_STATUS Status;
> UINTN Bytes;
> - BOOLEAN AddressingMode64;
> - BOOLEAN DataLength26;
> UINT32 AdmaMaxDataPerLine;
> UINT32 DescSize;
> VOID *AdmaDesc;
>
> - AddressingMode64 = FALSE;
> - DataLength26 = FALSE;
> AdmaMaxDataPerLine = ADMA_MAX_DATA_PER_LINE_16B;
> DescSize = sizeof (SD_MMC_HC_ADMA_32_DESC_LINE);
> AdmaDesc = NULL;
> @@ -1410,27 +1420,16 @@ BuildAdmaDescTable (
> PciIo = Trb->Private->PciIo;
>
> //
> - // Detect whether 64bit addressing is supported.
> - //
> - if (ControllerVer >= SD_MMC_HC_CTRL_VER_400) {
> - Status = SdMmcHcCheckMmioSet(PciIo, Trb->Slot, SD_MMC_HC_HOST_CTRL2, sizeof(UINT16),
> - SD_MMC_HC_V4_EN|SD_MMC_HC_64_ADDR_EN,
> SD_MMC_HC_V4_EN|SD_MMC_HC_64_ADDR_EN);
> - if (!EFI_ERROR (Status)) {
> - AddressingMode64 = TRUE;
> - DescSize = sizeof (SD_MMC_HC_ADMA_64_DESC_LINE);
> - }
> - }
> - //
> // Check for valid ranges in 32bit ADMA Descriptor Table
> //
> - if (!AddressingMode64 &&
> + if ((Trb->Mode == SdMmcAdma32bMode) &&
> ((Data >= 0x100000000ul) || ((Data + DataLen) > 0x100000000ul))) {
> return EFI_INVALID_PARAMETER;
> }
> //
> // Check address field alignment
> //
> - if (AddressingMode64) {
> + if (Trb->Mode != SdMmcAdma32bMode) {
> //
> // Address field shall be set on 64-bit boundary (Lower 3-bit is always set to 0)
> //
> @@ -1445,13 +1444,19 @@ BuildAdmaDescTable (
> DEBUG ((DEBUG_INFO, "The buffer [0x%x] to construct ADMA desc is not aligned to 4 bytes boundary!\n", Data));
> }
> }
> +
> //
> - // Detect whether 26bit data length is supported.
> + // Configure 64b ADMA.
> //
> - Status = SdMmcHcCheckMmioSet(PciIo, Trb->Slot, SD_MMC_HC_HOST_CTRL2, sizeof(UINT16),
> - SD_MMC_HC_26_DATA_LEN_ADMA_EN, SD_MMC_HC_26_DATA_LEN_ADMA_EN);
> - if (!EFI_ERROR (Status)) {
> - DataLength26 = TRUE;
> + if (Trb->Mode == SdMmcAdma64bV3Mode) {
> + DescSize = sizeof (SD_MMC_HC_ADMA_64_V3_DESC_LINE);
> + }else if (Trb->Mode == SdMmcAdma64bV4Mode) {
> + DescSize = sizeof (SD_MMC_HC_ADMA_64_V4_DESC_LINE);
> + }
> + //
> + // Configure 26b data length.
> + //
> + if (Trb->Length == SdMmcAdmaLen26b) {
> AdmaMaxDataPerLine = ADMA_MAX_DATA_PER_LINE_26B;
> }
>
> @@ -1492,7 +1497,7 @@ BuildAdmaDescTable (
> return EFI_OUT_OF_RESOURCES;
> }
>
> - if ((!AddressingMode64) &&
> + if ((Trb->Mode == SdMmcAdma32bMode) &&
> (UINT64)(UINTN)Trb->AdmaDescPhy > 0x100000000ul) {
> //
> // The ADMA doesn't support 64bit addressing.
> @@ -1511,19 +1516,26 @@ BuildAdmaDescTable (
>
> Remaining = DataLen;
> Address = Data;
> - if (!AddressingMode64) {
> + if (Trb->Mode == SdMmcAdma32bMode) {
> Trb->Adma32Desc = AdmaDesc;
> - Trb->Adma64Desc = NULL;
> + Trb->Adma64V3Desc = NULL;
> + Trb->Adma64V4Desc = NULL;
> + } else if (Trb->Mode == SdMmcAdma64bV3Mode) {
> + Trb->Adma64V3Desc = AdmaDesc;
> + Trb->Adma32Desc = NULL;
> + Trb->Adma64V4Desc = NULL;
> } else {
> - Trb->Adma64Desc = AdmaDesc;
> + Trb->Adma64V4Desc = AdmaDesc;
> Trb->Adma32Desc = NULL;
> + Trb->Adma64V3Desc = NULL;
> }
> +
> for (Index = 0; Index < Entries; Index++) {
> - if (!AddressingMode64) {
> + if (Trb->Mode == SdMmcAdma32bMode) {
> if (Remaining <= AdmaMaxDataPerLine) {
> Trb->Adma32Desc[Index].Valid = 1;
> Trb->Adma32Desc[Index].Act = 2;
> - if (DataLength26) {
> + if (Trb->Length == SdMmcAdmaLen26b) {
> Trb->Adma32Desc[Index].UpperLength = (UINT16)RShiftU64 (Remaining, 16);
> }
> Trb->Adma32Desc[Index].LowerLength = (UINT16)(Remaining & MAX_UINT16);
> @@ -1532,32 +1544,53 @@ BuildAdmaDescTable (
> } else {
> Trb->Adma32Desc[Index].Valid = 1;
> Trb->Adma32Desc[Index].Act = 2;
> - if (DataLength26) {
> + if (Trb->Length == SdMmcAdmaLen26b) {
> Trb->Adma32Desc[Index].UpperLength = 0;
> }
> Trb->Adma32Desc[Index].LowerLength = 0;
> Trb->Adma32Desc[Index].Address = (UINT32)Address;
> }
> + } else if (Trb->Mode == SdMmcAdma64bV3Mode) {
> + if (Remaining <= AdmaMaxDataPerLine) {
> + Trb->Adma64V3Desc[Index].Valid = 1;
> + Trb->Adma64V3Desc[Index].Act = 2;
> + if (Trb->Length == SdMmcAdmaLen26b) {
> + Trb->Adma64V3Desc[Index].UpperLength = (UINT16)RShiftU64 (Remaining, 16);
> + }
> + Trb->Adma64V3Desc[Index].LowerLength = (UINT16)(Remaining & MAX_UINT16);
> + Trb->Adma64V3Desc[Index].LowerAddress = (UINT32)Address;
> + Trb->Adma64V3Desc[Index].UpperAddress = (UINT32)RShiftU64 (Address, 32);
> + break;
> + } else {
> + Trb->Adma64V3Desc[Index].Valid = 1;
> + Trb->Adma64V3Desc[Index].Act = 2;
> + if (Trb->Length == SdMmcAdmaLen26b) {
> + Trb->Adma64V3Desc[Index].UpperLength = 0;
> + }
> + Trb->Adma64V3Desc[Index].LowerLength = 0;
> + Trb->Adma64V3Desc[Index].LowerAddress = (UINT32)Address;
> + Trb->Adma64V3Desc[Index].UpperAddress = (UINT32)RShiftU64 (Address, 32);
> + }
> } else {
> if (Remaining <= AdmaMaxDataPerLine) {
> - Trb->Adma64Desc[Index].Valid = 1;
> - Trb->Adma64Desc[Index].Act = 2;
> - if (DataLength26) {
> - Trb->Adma64Desc[Index].UpperLength = (UINT16)RShiftU64 (Remaining, 16);
> + Trb->Adma64V4Desc[Index].Valid = 1;
> + Trb->Adma64V4Desc[Index].Act = 2;
> + if (Trb->Length == SdMmcAdmaLen26b) {
> + Trb->Adma64V4Desc[Index].UpperLength = (UINT16)RShiftU64 (Remaining, 16);
> }
> - Trb->Adma64Desc[Index].LowerLength = (UINT16)(Remaining & MAX_UINT16);
> - Trb->Adma64Desc[Index].LowerAddress = (UINT32)Address;
> - Trb->Adma64Desc[Index].UpperAddress = (UINT32)RShiftU64 (Address, 32);
> + Trb->Adma64V4Desc[Index].LowerLength = (UINT16)(Remaining & MAX_UINT16);
> + Trb->Adma64V4Desc[Index].LowerAddress = (UINT32)Address;
> + Trb->Adma64V4Desc[Index].UpperAddress = (UINT32)RShiftU64 (Address, 32);
> break;
> } else {
> - Trb->Adma64Desc[Index].Valid = 1;
> - Trb->Adma64Desc[Index].Act = 2;
> - if (DataLength26) {
> - Trb->Adma64Desc[Index].UpperLength = 0;
> + Trb->Adma64V4Desc[Index].Valid = 1;
> + Trb->Adma64V4Desc[Index].Act = 2;
> + if (Trb->Length == SdMmcAdmaLen26b) {
> + Trb->Adma64V4Desc[Index].UpperLength = 0;
> }
> - Trb->Adma64Desc[Index].LowerLength = 0;
> - Trb->Adma64Desc[Index].LowerAddress = (UINT32)Address;
> - Trb->Adma64Desc[Index].UpperAddress = (UINT32)RShiftU64 (Address, 32);
> + Trb->Adma64V4Desc[Index].LowerLength = 0;
> + Trb->Adma64V4Desc[Index].LowerAddress = (UINT32)Address;
> + Trb->Adma64V4Desc[Index].UpperAddress = (UINT32)RShiftU64 (Address, 32);
> }
> }
>
> @@ -1568,7 +1601,13 @@ BuildAdmaDescTable (
> //
> // Set the last descriptor line as end of descriptor table
> //
> - AddressingMode64 ? (Trb->Adma64Desc[Index].End = 1) : (Trb->Adma32Desc[Index].End = 1);
> + if (Trb->Mode == SdMmcAdma32bMode) {
> + Trb->Adma32Desc[Index].End = 1;
> + } else if (Trb->Mode == SdMmcAdma64bV3Mode) {
> + Trb->Adma64V3Desc[Index].End = 1;
> + } else {
> + Trb->Adma64V4Desc[Index].End = 1;
> + }
> return EFI_SUCCESS;
> }
>
> @@ -1665,7 +1704,20 @@ SdMmcCreateTrb (
> if (Trb->DataLen == 0) {
> Trb->Mode = SdMmcNoData;
> } else if (Private->Capability[Slot].Adma2 != 0) {
> - Trb->Mode = SdMmcAdmaMode;
> + Trb->Mode = SdMmcAdma32bMode;
> + Trb->Length = SdMmcAdmaLen16b;
> + if ((Private->ControllerVersion[Slot] == SD_MMC_HC_CTRL_VER_300) &&
> + (Private->Capability[Slot].SysBus64V3 == 1)) {
> + Trb->Mode = SdMmcAdma64bV3Mode;
> + } else if (((Private->ControllerVersion[Slot] == SD_MMC_HC_CTRL_VER_400) &&
> + (Private->Capability[Slot].SysBus64V3 == 1)) ||
> + ((Private->ControllerVersion[Slot] >= SD_MMC_HC_CTRL_VER_410) &&
> + (Private->Capability[Slot].SysBus64V4 == 1))) {
> + Trb->Mode = SdMmcAdma64bV4Mode;
> + }
> + if (Private->ControllerVersion[Slot] >= SD_MMC_HC_CTRL_VER_410) {
> + Trb->Length = SdMmcAdmaLen26b;
> + }
> Status = BuildAdmaDescTable (Trb, Private->ControllerVersion[Slot]);
> if (EFI_ERROR (Status)) {
> PciIo->Unmap (PciIo, Trb->DataMap);
> @@ -1719,11 +1771,18 @@ SdMmcFreeTrb (
> Trb->Adma32Desc
> );
> }
> - if (Trb->Adma64Desc != NULL) {
> + if (Trb->Adma64V3Desc != NULL) {
> + PciIo->FreeBuffer (
> + PciIo,
> + Trb->AdmaPages,
> + Trb->Adma64V3Desc
> + );
> + }
> + if (Trb->Adma64V4Desc != NULL) {
> PciIo->FreeBuffer (
> PciIo,
> Trb->AdmaPages,
> - Trb->Adma64Desc
> + Trb->Adma64V4Desc
> );
> }
> if (Trb->DataMap != NULL) {
> @@ -1891,27 +1950,35 @@ SdMmcExecTrb (
> if (EFI_ERROR (Status)) {
> return Status;
> }
> +
> + if (Private->ControllerVersion[Trb->Slot] >= SD_MMC_HC_CTRL_VER_400) {
> + Status = SdMmcHcCheckMmioSet(PciIo, Trb->Slot, SD_MMC_HC_HOST_CTRL2, sizeof(UINT16),
> + SD_MMC_HC_64_ADDR_EN, SD_MMC_HC_64_ADDR_EN);
> + if (!EFI_ERROR (Status)) {
> + AddressingMode64 = TRUE;
> + }
> + }
> +
> //
> // Set Host Control 1 register DMA Select field
> //
> - if (Trb->Mode == SdMmcAdmaMode) {
> + if ((Trb->Mode == SdMmcAdma32bMode) ||
> + (Trb->Mode == SdMmcAdma64bV4Mode)) {
> HostCtrl1 = BIT4;
> Status = SdMmcHcOrMmio (PciIo, Trb->Slot, SD_MMC_HC_HOST_CTRL1, sizeof (HostCtrl1), &HostCtrl1);
> if (EFI_ERROR (Status)) {
> return Status;
> }
> + } else if (Trb->Mode == SdMmcAdma64bV3Mode) {
> + HostCtrl1 = BIT4|BIT3;
> + Status = SdMmcHcOrMmio (PciIo, Trb->Slot, SD_MMC_HC_HOST_CTRL1, sizeof (HostCtrl1), &HostCtrl1);
> + if (EFI_ERROR (Status)) {
> + return Status;
> + }
> }
>
> SdMmcHcLedOnOff (PciIo, Trb->Slot, TRUE);
>
> - if (Private->ControllerVersion[Trb->Slot] >= SD_MMC_HC_CTRL_VER_400) {
> - Status = SdMmcHcCheckMmioSet(PciIo, Trb->Slot, SD_MMC_HC_HOST_CTRL2, sizeof(UINT16),
> - SD_MMC_HC_V4_EN|SD_MMC_HC_64_ADDR_EN,
> SD_MMC_HC_V4_EN|SD_MMC_HC_64_ADDR_EN);
> - if (!EFI_ERROR (Status)) {
> - AddressingMode64 = TRUE;
> - }
> - }
> -
> if (Trb->Mode == SdMmcSdmaMode) {
> if ((!AddressingMode64) &&
> ((UINT64)(UINTN)Trb->DataPhy >= 0x100000000ul)) {
> @@ -1929,7 +1996,9 @@ SdMmcExecTrb (
> if (EFI_ERROR (Status)) {
> return Status;
> }
> - } else if (Trb->Mode == SdMmcAdmaMode) {
> + } else if ((Trb->Mode == SdMmcAdma32bMode) ||
> + (Trb->Mode == SdMmcAdma64bV3Mode) ||
> + (Trb->Mode == SdMmcAdma64bV4Mode)) {
> AdmaAddr = (UINT64)(UINTN)Trb->AdmaDescPhy;
> Status = SdMmcHcRwMmio (PciIo, Trb->Slot, SD_MMC_HC_ADMA_SYS_ADDR, FALSE, sizeof (AdmaAddr), &AdmaAddr);
> if (EFI_ERROR (Status)) {
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> index d157f2c..3a05456 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> @@ -2,7 +2,7 @@
>
> Provides some data structure definitions used by the SD/MMC host controller driver.
>
> -Copyright (c) 2018, NVIDIA CORPORATION. All rights reserved.
> +Copyright (c) 2018-2019, NVIDIA CORPORATION. All rights reserved.
> Copyright (c) 2015, Intel Corporation. All rights reserved.<BR>
> This program and the accompanying materials
> are licensed and made available under the terms and conditions of the BSD License
> @@ -80,16 +80,25 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
>
> //
> // The transfer modes supported by SD Host Controller
> -// Simplified Spec 3.0 Table 1-2
> //
> typedef enum {
> SdMmcNoData,
> SdMmcPioMode,
> SdMmcSdmaMode,
> - SdMmcAdmaMode
> + SdMmcAdma32bMode,
> + SdMmcAdma64bV3Mode,
> + SdMmcAdma64bV4Mode
> } SD_MMC_HC_TRANSFER_MODE;
>
> //
> +// The ADMA transfer lengths supported by SD Host Controller
> +//
> +typedef enum {
> + SdMmcAdmaLen16b,
> + SdMmcAdmaLen26b
> +} SD_MMC_HC_ADMA_LEGTH;
> +
> +//
> // The maximum data length of each descriptor line
> //
> #define ADMA_MAX_DATA_PER_LINE_16B SIZE_64KB
> @@ -122,8 +131,20 @@ typedef struct {
> UINT32 LowerLength:16;
> UINT32 LowerAddress;
> UINT32 UpperAddress;
> +} SD_MMC_HC_ADMA_64_V3_DESC_LINE;
> +
> +typedef struct {
> + UINT32 Valid:1;
> + UINT32 End:1;
> + UINT32 Int:1;
> + UINT32 Reserved:1;
> + UINT32 Act:2;
> + UINT32 UpperLength:10;
> + UINT32 LowerLength:16;
> + UINT32 LowerAddress;
> + UINT32 UpperAddress;
> UINT32 Reserved1;
> -} SD_MMC_HC_ADMA_64_DESC_LINE;
> +} SD_MMC_HC_ADMA_64_V4_DESC_LINE;
>
> #define SD_MMC_SDMA_BOUNDARY 512 * 1024
> #define SD_MMC_SDMA_ROUND_UP(x, n) (((x) + n) & ~(n - 1))
> --
> 2.7.4
>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] MdeModulePkg/SdMmcPciHcDxe: Add V3 64b DMA Support
2019-03-02 16:00 ` Gao, Liming
@ 2019-03-04 4:08 ` Ashish Singhal
0 siblings, 0 replies; 9+ messages in thread
From: Ashish Singhal @ 2019-03-04 4:08 UTC (permalink / raw)
To: Gao, Liming, edk2-devel@lists.01.org; +Cc: Wu, Hao A
Liming,
I am OK waiting till the freeze is over.
Thanks
Ashish
-----Original Message-----
From: Gao, Liming <liming.gao@intel.com>
Sent: Saturday, March 2, 2019 9:01 AM
To: Ashish Singhal <ashishsingha@nvidia.com>; edk2-devel@lists.01.org
Cc: Wu, Hao A <hao.a.wu@intel.com>
Subject: RE: [edk2] [PATCH] MdeModulePkg/SdMmcPciHcDxe: Add V3 64b DMA Support
Ashish:
This seems a feature. Now, we are in Hard Feature Freeze. So, it will not be added until edk2-stable201903 tag is created at 2019-03-08.
Thanks
Liming
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Ashish Singhal
> Sent: Saturday, March 2, 2019 2:30 AM
> To: edk2-devel@lists.01.org
> Cc: Wu, Hao A <hao.a.wu@intel.com>; Ashish Singhal
> <ashishsingha@nvidia.com>
> Subject: [edk2] [PATCH] MdeModulePkg/SdMmcPciHcDxe: Add V3 64b DMA
> Support
>
> Driver was supporting only 32b DMA support for V3 controllers. Add
> support for 64b DMA as well for completeness.
>
> For V4.0 64b support, driver was looking at incorrect capability
> register bit. Fix for that is present as well.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ashish Singhal <ashishsingha@nvidia.com>
> ---
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c | 10 +-
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h | 6 +-
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c | 199 ++++++++++++++-------
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h | 29 ++-
> 4 files changed, 170 insertions(+), 74 deletions(-)
>
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> index b474f8d..9b7b88c 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> @@ -6,7 +6,7 @@
>
> It would expose EFI_SD_MMC_PASS_THRU_PROTOCOL for upper layer use.
>
> - Copyright (c) 2018, NVIDIA CORPORATION. All rights reserved.
> + Copyright (c) 2018-2019, NVIDIA CORPORATION. All rights reserved.
> Copyright (c) 2015 - 2019, Intel Corporation. All rights reserved.<BR>
> This program and the accompanying materials
> are licensed and made available under the terms and conditions of
> the BSD License @@ -666,8 +666,12 @@ SdMmcPciHcDriverBindingStart (
> // If any of the slots does not support 64b system bus
> // do not enable 64b DMA in the PCI layer.
> //
> - if (Private->Capability[Slot].SysBus64V3 == 0 &&
> - Private->Capability[Slot].SysBus64V4 == 0) {
> + if ((Private->ControllerVersion[Slot] == SD_MMC_HC_CTRL_VER_300 &&
> + Private->Capability[Slot].SysBus64V3 == 0) ||
> + (Private->ControllerVersion[Slot] == SD_MMC_HC_CTRL_VER_400 &&
> + Private->Capability[Slot].SysBus64V3 == 0) ||
> + (Private->ControllerVersion[Slot] >= SD_MMC_HC_CTRL_VER_410 &&
> + Private->Capability[Slot].SysBus64V4 == 0)) {
> Support64BitDma = FALSE;
> }
>
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> index 1bb701a..68d8a5c 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> @@ -2,7 +2,7 @@
>
> Provides some data structure definitions used by the SD/MMC host controller driver.
>
> -Copyright (c) 2018, NVIDIA CORPORATION. All rights reserved.
> +Copyright (c) 2018-2019, NVIDIA CORPORATION. All rights reserved.
> Copyright (c) 2015, Intel Corporation. All rights reserved.<BR> This
> program and the accompanying materials are licensed and made
> available under the terms and conditions of the BSD License @@ -145,13
> +145,15 @@ typedef struct {
> EFI_PHYSICAL_ADDRESS DataPhy;
> VOID *DataMap;
> SD_MMC_HC_TRANSFER_MODE Mode;
> + SD_MMC_HC_ADMA_LEGTH Length;
>
> EFI_EVENT Event;
> BOOLEAN Started;
> UINT64 Timeout;
>
> SD_MMC_HC_ADMA_32_DESC_LINE *Adma32Desc;
> - SD_MMC_HC_ADMA_64_DESC_LINE *Adma64Desc;
> + SD_MMC_HC_ADMA_64_V3_DESC_LINE *Adma64V3Desc;
> + SD_MMC_HC_ADMA_64_V4_DESC_LINE *Adma64V4Desc;
> EFI_PHYSICAL_ADDRESS AdmaDescPhy;
> VOID *AdmaMap;
> UINT32 AdmaPages;
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> index d73fa10..a6d2395 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> @@ -6,7 +6,7 @@
>
> It would expose EFI_SD_MMC_PASS_THRU_PROTOCOL for upper layer use.
>
> - Copyright (c) 2018, NVIDIA CORPORATION. All rights reserved.
> + Copyright (c) 2018-2019, NVIDIA CORPORATION. All rights reserved.
> Copyright (c) 2015 - 2019, Intel Corporation. All rights reserved.<BR>
> This program and the accompanying materials
> are licensed and made available under the terms and conditions of
> the BSD License @@ -1010,18 +1010,32 @@ SdMmcHcInitV4Enhancements (
> if (ControllerVer >= SD_MMC_HC_CTRL_VER_400) {
> HostCtrl2 = SD_MMC_HC_V4_EN;
> //
> - // Check if V4 64bit support is available
> + // Check if controller version V4.0
> //
> - if (Capability.SysBus64V4 != 0) {
> - HostCtrl2 |= SD_MMC_HC_64_ADDR_EN;
> - DEBUG ((DEBUG_INFO, "Enabled V4 64 bit system bus support\n"));
> + if (ControllerVer == SD_MMC_HC_CTRL_VER_400) {
> + //
> + // Check if 64bit support is available
> + //
> + if (Capability.SysBus64V3 != 0) {
> + HostCtrl2 |= SD_MMC_HC_64_ADDR_EN;
> + DEBUG ((DEBUG_INFO, "Enabled V4 64 bit system bus support\n"));
> + }
> }
> //
> // Check if controller version V4.10 or higher
> //
> - if (ControllerVer >= SD_MMC_HC_CTRL_VER_410) {
> - HostCtrl2 |= SD_MMC_HC_26_DATA_LEN_ADMA_EN;
> - DEBUG ((DEBUG_INFO, "Enabled V4 26 bit data length ADMA support\n"));
> + else if (ControllerVer >= SD_MMC_HC_CTRL_VER_410) {
> + //
> + // Check if 64bit support is available
> + //
> + if (Capability.SysBus64V4 != 0) {
> + HostCtrl2 |= SD_MMC_HC_64_ADDR_EN;
> + DEBUG ((DEBUG_INFO, "Enabled V4 64 bit system bus support\n"));
> + }
> + if (ControllerVer >= SD_MMC_HC_CTRL_VER_410) {
> + HostCtrl2 |= SD_MMC_HC_26_DATA_LEN_ADMA_EN;
> + DEBUG ((DEBUG_INFO, "Enabled V4 26 bit data length ADMA support\n"));
> + }
> }
> Status = SdMmcHcOrMmio (PciIo, Slot, SD_MMC_HC_HOST_CTRL2, sizeof (HostCtrl2), &HostCtrl2);
> if (EFI_ERROR (Status)) {
> @@ -1393,14 +1407,10 @@ BuildAdmaDescTable (
> EFI_PCI_IO_PROTOCOL *PciIo;
> EFI_STATUS Status;
> UINTN Bytes;
> - BOOLEAN AddressingMode64;
> - BOOLEAN DataLength26;
> UINT32 AdmaMaxDataPerLine;
> UINT32 DescSize;
> VOID *AdmaDesc;
>
> - AddressingMode64 = FALSE;
> - DataLength26 = FALSE;
> AdmaMaxDataPerLine = ADMA_MAX_DATA_PER_LINE_16B;
> DescSize = sizeof (SD_MMC_HC_ADMA_32_DESC_LINE);
> AdmaDesc = NULL;
> @@ -1410,27 +1420,16 @@ BuildAdmaDescTable (
> PciIo = Trb->Private->PciIo;
>
> //
> - // Detect whether 64bit addressing is supported.
> - //
> - if (ControllerVer >= SD_MMC_HC_CTRL_VER_400) {
> - Status = SdMmcHcCheckMmioSet(PciIo, Trb->Slot, SD_MMC_HC_HOST_CTRL2, sizeof(UINT16),
> - SD_MMC_HC_V4_EN|SD_MMC_HC_64_ADDR_EN,
> SD_MMC_HC_V4_EN|SD_MMC_HC_64_ADDR_EN);
> - if (!EFI_ERROR (Status)) {
> - AddressingMode64 = TRUE;
> - DescSize = sizeof (SD_MMC_HC_ADMA_64_DESC_LINE);
> - }
> - }
> - //
> // Check for valid ranges in 32bit ADMA Descriptor Table
> //
> - if (!AddressingMode64 &&
> + if ((Trb->Mode == SdMmcAdma32bMode) &&
> ((Data >= 0x100000000ul) || ((Data + DataLen) > 0x100000000ul))) {
> return EFI_INVALID_PARAMETER;
> }
> //
> // Check address field alignment
> //
> - if (AddressingMode64) {
> + if (Trb->Mode != SdMmcAdma32bMode) {
> //
> // Address field shall be set on 64-bit boundary (Lower 3-bit is always set to 0)
> //
> @@ -1445,13 +1444,19 @@ BuildAdmaDescTable (
> DEBUG ((DEBUG_INFO, "The buffer [0x%x] to construct ADMA desc is not aligned to 4 bytes boundary!\n", Data));
> }
> }
> +
> //
> - // Detect whether 26bit data length is supported.
> + // Configure 64b ADMA.
> //
> - Status = SdMmcHcCheckMmioSet(PciIo, Trb->Slot, SD_MMC_HC_HOST_CTRL2, sizeof(UINT16),
> - SD_MMC_HC_26_DATA_LEN_ADMA_EN, SD_MMC_HC_26_DATA_LEN_ADMA_EN);
> - if (!EFI_ERROR (Status)) {
> - DataLength26 = TRUE;
> + if (Trb->Mode == SdMmcAdma64bV3Mode) {
> + DescSize = sizeof (SD_MMC_HC_ADMA_64_V3_DESC_LINE); }else if
> + (Trb->Mode == SdMmcAdma64bV4Mode) {
> + DescSize = sizeof (SD_MMC_HC_ADMA_64_V4_DESC_LINE); } // //
> + Configure 26b data length.
> + //
> + if (Trb->Length == SdMmcAdmaLen26b) {
> AdmaMaxDataPerLine = ADMA_MAX_DATA_PER_LINE_26B;
> }
>
> @@ -1492,7 +1497,7 @@ BuildAdmaDescTable (
> return EFI_OUT_OF_RESOURCES;
> }
>
> - if ((!AddressingMode64) &&
> + if ((Trb->Mode == SdMmcAdma32bMode) &&
> (UINT64)(UINTN)Trb->AdmaDescPhy > 0x100000000ul) {
> //
> // The ADMA doesn't support 64bit addressing.
> @@ -1511,19 +1516,26 @@ BuildAdmaDescTable (
>
> Remaining = DataLen;
> Address = Data;
> - if (!AddressingMode64) {
> + if (Trb->Mode == SdMmcAdma32bMode) {
> Trb->Adma32Desc = AdmaDesc;
> - Trb->Adma64Desc = NULL;
> + Trb->Adma64V3Desc = NULL;
> + Trb->Adma64V4Desc = NULL;
> + } else if (Trb->Mode == SdMmcAdma64bV3Mode) {
> + Trb->Adma64V3Desc = AdmaDesc;
> + Trb->Adma32Desc = NULL;
> + Trb->Adma64V4Desc = NULL;
> } else {
> - Trb->Adma64Desc = AdmaDesc;
> + Trb->Adma64V4Desc = AdmaDesc;
> Trb->Adma32Desc = NULL;
> + Trb->Adma64V3Desc = NULL;
> }
> +
> for (Index = 0; Index < Entries; Index++) {
> - if (!AddressingMode64) {
> + if (Trb->Mode == SdMmcAdma32bMode) {
> if (Remaining <= AdmaMaxDataPerLine) {
> Trb->Adma32Desc[Index].Valid = 1;
> Trb->Adma32Desc[Index].Act = 2;
> - if (DataLength26) {
> + if (Trb->Length == SdMmcAdmaLen26b) {
> Trb->Adma32Desc[Index].UpperLength = (UINT16)RShiftU64 (Remaining, 16);
> }
> Trb->Adma32Desc[Index].LowerLength = (UINT16)(Remaining &
> MAX_UINT16); @@ -1532,32 +1544,53 @@ BuildAdmaDescTable (
> } else {
> Trb->Adma32Desc[Index].Valid = 1;
> Trb->Adma32Desc[Index].Act = 2;
> - if (DataLength26) {
> + if (Trb->Length == SdMmcAdmaLen26b) {
> Trb->Adma32Desc[Index].UpperLength = 0;
> }
> Trb->Adma32Desc[Index].LowerLength = 0;
> Trb->Adma32Desc[Index].Address = (UINT32)Address;
> }
> + } else if (Trb->Mode == SdMmcAdma64bV3Mode) {
> + if (Remaining <= AdmaMaxDataPerLine) {
> + Trb->Adma64V3Desc[Index].Valid = 1;
> + Trb->Adma64V3Desc[Index].Act = 2;
> + if (Trb->Length == SdMmcAdmaLen26b) {
> + Trb->Adma64V3Desc[Index].UpperLength = (UINT16)RShiftU64 (Remaining, 16);
> + }
> + Trb->Adma64V3Desc[Index].LowerLength = (UINT16)(Remaining & MAX_UINT16);
> + Trb->Adma64V3Desc[Index].LowerAddress = (UINT32)Address;
> + Trb->Adma64V3Desc[Index].UpperAddress = (UINT32)RShiftU64 (Address, 32);
> + break;
> + } else {
> + Trb->Adma64V3Desc[Index].Valid = 1;
> + Trb->Adma64V3Desc[Index].Act = 2;
> + if (Trb->Length == SdMmcAdmaLen26b) {
> + Trb->Adma64V3Desc[Index].UpperLength = 0;
> + }
> + Trb->Adma64V3Desc[Index].LowerLength = 0;
> + Trb->Adma64V3Desc[Index].LowerAddress = (UINT32)Address;
> + Trb->Adma64V3Desc[Index].UpperAddress = (UINT32)RShiftU64 (Address, 32);
> + }
> } else {
> if (Remaining <= AdmaMaxDataPerLine) {
> - Trb->Adma64Desc[Index].Valid = 1;
> - Trb->Adma64Desc[Index].Act = 2;
> - if (DataLength26) {
> - Trb->Adma64Desc[Index].UpperLength = (UINT16)RShiftU64 (Remaining, 16);
> + Trb->Adma64V4Desc[Index].Valid = 1;
> + Trb->Adma64V4Desc[Index].Act = 2;
> + if (Trb->Length == SdMmcAdmaLen26b) {
> + Trb->Adma64V4Desc[Index].UpperLength = (UINT16)RShiftU64
> + (Remaining, 16);
> }
> - Trb->Adma64Desc[Index].LowerLength = (UINT16)(Remaining & MAX_UINT16);
> - Trb->Adma64Desc[Index].LowerAddress = (UINT32)Address;
> - Trb->Adma64Desc[Index].UpperAddress = (UINT32)RShiftU64 (Address, 32);
> + Trb->Adma64V4Desc[Index].LowerLength = (UINT16)(Remaining & MAX_UINT16);
> + Trb->Adma64V4Desc[Index].LowerAddress = (UINT32)Address;
> + Trb->Adma64V4Desc[Index].UpperAddress = (UINT32)RShiftU64
> + (Address, 32);
> break;
> } else {
> - Trb->Adma64Desc[Index].Valid = 1;
> - Trb->Adma64Desc[Index].Act = 2;
> - if (DataLength26) {
> - Trb->Adma64Desc[Index].UpperLength = 0;
> + Trb->Adma64V4Desc[Index].Valid = 1;
> + Trb->Adma64V4Desc[Index].Act = 2;
> + if (Trb->Length == SdMmcAdmaLen26b) {
> + Trb->Adma64V4Desc[Index].UpperLength = 0;
> }
> - Trb->Adma64Desc[Index].LowerLength = 0;
> - Trb->Adma64Desc[Index].LowerAddress = (UINT32)Address;
> - Trb->Adma64Desc[Index].UpperAddress = (UINT32)RShiftU64 (Address, 32);
> + Trb->Adma64V4Desc[Index].LowerLength = 0;
> + Trb->Adma64V4Desc[Index].LowerAddress = (UINT32)Address;
> + Trb->Adma64V4Desc[Index].UpperAddress = (UINT32)RShiftU64
> + (Address, 32);
> }
> }
>
> @@ -1568,7 +1601,13 @@ BuildAdmaDescTable (
> //
> // Set the last descriptor line as end of descriptor table
> //
> - AddressingMode64 ? (Trb->Adma64Desc[Index].End = 1) :
> (Trb->Adma32Desc[Index].End = 1);
> + if (Trb->Mode == SdMmcAdma32bMode) {
> + Trb->Adma32Desc[Index].End = 1;
> + } else if (Trb->Mode == SdMmcAdma64bV3Mode) {
> + Trb->Adma64V3Desc[Index].End = 1; } else {
> + Trb->Adma64V4Desc[Index].End = 1; }
> return EFI_SUCCESS;
> }
>
> @@ -1665,7 +1704,20 @@ SdMmcCreateTrb (
> if (Trb->DataLen == 0) {
> Trb->Mode = SdMmcNoData;
> } else if (Private->Capability[Slot].Adma2 != 0) {
> - Trb->Mode = SdMmcAdmaMode;
> + Trb->Mode = SdMmcAdma32bMode;
> + Trb->Length = SdMmcAdmaLen16b;
> + if ((Private->ControllerVersion[Slot] == SD_MMC_HC_CTRL_VER_300) &&
> + (Private->Capability[Slot].SysBus64V3 == 1)) {
> + Trb->Mode = SdMmcAdma64bV3Mode;
> + } else if (((Private->ControllerVersion[Slot] == SD_MMC_HC_CTRL_VER_400) &&
> + (Private->Capability[Slot].SysBus64V3 == 1)) ||
> + ((Private->ControllerVersion[Slot] >= SD_MMC_HC_CTRL_VER_410) &&
> + (Private->Capability[Slot].SysBus64V4 == 1))) {
> + Trb->Mode = SdMmcAdma64bV4Mode;
> + }
> + if (Private->ControllerVersion[Slot] >= SD_MMC_HC_CTRL_VER_410) {
> + Trb->Length = SdMmcAdmaLen26b;
> + }
> Status = BuildAdmaDescTable (Trb, Private->ControllerVersion[Slot]);
> if (EFI_ERROR (Status)) {
> PciIo->Unmap (PciIo, Trb->DataMap); @@ -1719,11 +1771,18 @@
> SdMmcFreeTrb (
> Trb->Adma32Desc
> );
> }
> - if (Trb->Adma64Desc != NULL) {
> + if (Trb->Adma64V3Desc != NULL) {
> + PciIo->FreeBuffer (
> + PciIo,
> + Trb->AdmaPages,
> + Trb->Adma64V3Desc
> + );
> + }
> + if (Trb->Adma64V4Desc != NULL) {
> PciIo->FreeBuffer (
> PciIo,
> Trb->AdmaPages,
> - Trb->Adma64Desc
> + Trb->Adma64V4Desc
> );
> }
> if (Trb->DataMap != NULL) {
> @@ -1891,27 +1950,35 @@ SdMmcExecTrb (
> if (EFI_ERROR (Status)) {
> return Status;
> }
> +
> + if (Private->ControllerVersion[Trb->Slot] >= SD_MMC_HC_CTRL_VER_400) {
> + Status = SdMmcHcCheckMmioSet(PciIo, Trb->Slot, SD_MMC_HC_HOST_CTRL2, sizeof(UINT16),
> + SD_MMC_HC_64_ADDR_EN, SD_MMC_HC_64_ADDR_EN);
> + if (!EFI_ERROR (Status)) {
> + AddressingMode64 = TRUE;
> + }
> + }
> +
> //
> // Set Host Control 1 register DMA Select field
> //
> - if (Trb->Mode == SdMmcAdmaMode) {
> + if ((Trb->Mode == SdMmcAdma32bMode) ||
> + (Trb->Mode == SdMmcAdma64bV4Mode)) {
> HostCtrl1 = BIT4;
> Status = SdMmcHcOrMmio (PciIo, Trb->Slot, SD_MMC_HC_HOST_CTRL1, sizeof (HostCtrl1), &HostCtrl1);
> if (EFI_ERROR (Status)) {
> return Status;
> }
> + } else if (Trb->Mode == SdMmcAdma64bV3Mode) {
> + HostCtrl1 = BIT4|BIT3;
> + Status = SdMmcHcOrMmio (PciIo, Trb->Slot, SD_MMC_HC_HOST_CTRL1, sizeof (HostCtrl1), &HostCtrl1);
> + if (EFI_ERROR (Status)) {
> + return Status;
> + }
> }
>
> SdMmcHcLedOnOff (PciIo, Trb->Slot, TRUE);
>
> - if (Private->ControllerVersion[Trb->Slot] >= SD_MMC_HC_CTRL_VER_400) {
> - Status = SdMmcHcCheckMmioSet(PciIo, Trb->Slot, SD_MMC_HC_HOST_CTRL2, sizeof(UINT16),
> - SD_MMC_HC_V4_EN|SD_MMC_HC_64_ADDR_EN,
> SD_MMC_HC_V4_EN|SD_MMC_HC_64_ADDR_EN);
> - if (!EFI_ERROR (Status)) {
> - AddressingMode64 = TRUE;
> - }
> - }
> -
> if (Trb->Mode == SdMmcSdmaMode) {
> if ((!AddressingMode64) &&
> ((UINT64)(UINTN)Trb->DataPhy >= 0x100000000ul)) { @@ -1929,7
> +1996,9 @@ SdMmcExecTrb (
> if (EFI_ERROR (Status)) {
> return Status;
> }
> - } else if (Trb->Mode == SdMmcAdmaMode) {
> + } else if ((Trb->Mode == SdMmcAdma32bMode) ||
> + (Trb->Mode == SdMmcAdma64bV3Mode) ||
> + (Trb->Mode == SdMmcAdma64bV4Mode)) {
> AdmaAddr = (UINT64)(UINTN)Trb->AdmaDescPhy;
> Status = SdMmcHcRwMmio (PciIo, Trb->Slot, SD_MMC_HC_ADMA_SYS_ADDR, FALSE, sizeof (AdmaAddr), &AdmaAddr);
> if (EFI_ERROR (Status)) {
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> index d157f2c..3a05456 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> @@ -2,7 +2,7 @@
>
> Provides some data structure definitions used by the SD/MMC host controller driver.
>
> -Copyright (c) 2018, NVIDIA CORPORATION. All rights reserved.
> +Copyright (c) 2018-2019, NVIDIA CORPORATION. All rights reserved.
> Copyright (c) 2015, Intel Corporation. All rights reserved.<BR> This
> program and the accompanying materials are licensed and made
> available under the terms and conditions of the BSD License @@ -80,16
> +80,25 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
>
> //
> // The transfer modes supported by SD Host Controller -// Simplified
> Spec 3.0 Table 1-2 // typedef enum {
> SdMmcNoData,
> SdMmcPioMode,
> SdMmcSdmaMode,
> - SdMmcAdmaMode
> + SdMmcAdma32bMode,
> + SdMmcAdma64bV3Mode,
> + SdMmcAdma64bV4Mode
> } SD_MMC_HC_TRANSFER_MODE;
>
> //
> +// The ADMA transfer lengths supported by SD Host Controller //
> +typedef enum {
> + SdMmcAdmaLen16b,
> + SdMmcAdmaLen26b
> +} SD_MMC_HC_ADMA_LEGTH;
> +
> +//
> // The maximum data length of each descriptor line //
> #define ADMA_MAX_DATA_PER_LINE_16B SIZE_64KB
> @@ -122,8 +131,20 @@ typedef struct {
> UINT32 LowerLength:16;
> UINT32 LowerAddress;
> UINT32 UpperAddress;
> +} SD_MMC_HC_ADMA_64_V3_DESC_LINE;
> +
> +typedef struct {
> + UINT32 Valid:1;
> + UINT32 End:1;
> + UINT32 Int:1;
> + UINT32 Reserved:1;
> + UINT32 Act:2;
> + UINT32 UpperLength:10;
> + UINT32 LowerLength:16;
> + UINT32 LowerAddress;
> + UINT32 UpperAddress;
> UINT32 Reserved1;
> -} SD_MMC_HC_ADMA_64_DESC_LINE;
> +} SD_MMC_HC_ADMA_64_V4_DESC_LINE;
>
> #define SD_MMC_SDMA_BOUNDARY 512 * 1024
> #define SD_MMC_SDMA_ROUND_UP(x, n) (((x) + n) & ~(n - 1))
> --
> 2.7.4
>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information. Any unauthorized review, use, disclosure or distribution
is prohibited. If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] MdeModulePkg/SdMmcPciHcDxe: Add V3 64b DMA Support
2019-03-01 18:30 [PATCH] MdeModulePkg/SdMmcPciHcDxe: Add V3 64b DMA Support Ashish Singhal
2019-03-02 16:00 ` Gao, Liming
@ 2019-03-06 3:01 ` Wu, Hao A
2019-03-06 5:17 ` Ashish Singhal
2019-03-06 23:05 ` Cohen, Eugene
1 sibling, 2 replies; 9+ messages in thread
From: Wu, Hao A @ 2019-03-06 3:01 UTC (permalink / raw)
To: Ashish Singhal, edk2-devel@lists.01.org
Hi Ashish,
One thing to confirm, for the updated checks within
SdMmcPciHcDriverBindingStart():
> if ((Private->ControllerVersion[Slot] == SD_MMC_HC_CTRL_VER_300 &&
> Private->Capability[Slot].SysBus64V3 == 0) ||
> (Private->ControllerVersion[Slot] == SD_MMC_HC_CTRL_VER_400 &&
> Private->Capability[Slot].SysBus64V3 == 0) ||
> (Private->ControllerVersion[Slot] >= SD_MMC_HC_CTRL_VER_410 &&
> Private->Capability[Slot].SysBus64V4 == 0)) {
> Support64BitDma = FALSE;
> }
When the SDHC with version greater than 4.10, the check is only performed
against the 'SysBus64V4' bit. My understanding of the purpose is that:
1. For SDHC with version 4.00, the support of V3 mode and V4 mode of
64-bit System Address are reflect by bit 'SysBus64V3'. Thus, I can infer
that the possible support case is both or neither.
2. The spec states that SDHC with version greater than 4.10 divides the V3
mode and V4 mode support into 2 bits (SysBus64V3, SysBus64V4) so that the
V3 mode support can be optional.
So based on 1 & 2, we do not even bother to check the 'SysBus64V3' bit
when HC version >= 4.10. Is that right?
I verified the patch on SDHC version 3.00 with 64-bit System Address
Support. Hope more configurations are available for testing on Eugene's
side.
Besides, some minor comments below:
> -----Original Message-----
> From: Ashish Singhal [mailto:ashishsingha@nvidia.com]
> Sent: Saturday, March 02, 2019 2:30 AM
> To: edk2-devel@lists.01.org
> Cc: Wu, Hao A; eugene@hp.com; Ashish Singhal
> Subject: [PATCH] MdeModulePkg/SdMmcPciHcDxe: Add V3 64b DMA
> Support
Please help to add the below Bugzilla tracker for reference:
https://bugzilla.tianocore.org/show_bug.cgi?id=1583
I have updated the above tracker to match the purpose of the proposed
patch.
>
> Driver was supporting only 32b DMA support for V3 controllers. Add
> support for 64b DMA as well for completeness.
>
> For V4.0 64b support, driver was looking at incorrect capability
> register bit. Fix for that is present as well.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ashish Singhal <ashishsingha@nvidia.com>
> ---
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c | 10 +-
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h | 6 +-
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c | 199
> ++++++++++++++-------
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h | 29 ++-
> 4 files changed, 170 insertions(+), 74 deletions(-)
>
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> index b474f8d..9b7b88c 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> @@ -6,7 +6,7 @@
>
> It would expose EFI_SD_MMC_PASS_THRU_PROTOCOL for upper layer use.
>
> - Copyright (c) 2018, NVIDIA CORPORATION. All rights reserved.
> + Copyright (c) 2018-2019, NVIDIA CORPORATION. All rights reserved.
> Copyright (c) 2015 - 2019, Intel Corporation. All rights reserved.<BR>
> This program and the accompanying materials
> are licensed and made available under the terms and conditions of the BSD
> License
> @@ -666,8 +666,12 @@ SdMmcPciHcDriverBindingStart (
> // If any of the slots does not support 64b system bus
> // do not enable 64b DMA in the PCI layer.
> //
> - if (Private->Capability[Slot].SysBus64V3 == 0 &&
> - Private->Capability[Slot].SysBus64V4 == 0) {
> + if ((Private->ControllerVersion[Slot] == SD_MMC_HC_CTRL_VER_300 &&
> + Private->Capability[Slot].SysBus64V3 == 0) ||
> + (Private->ControllerVersion[Slot] == SD_MMC_HC_CTRL_VER_400 &&
> + Private->Capability[Slot].SysBus64V3 == 0) ||
> + (Private->ControllerVersion[Slot] >= SD_MMC_HC_CTRL_VER_410 &&
> + Private->Capability[Slot].SysBus64V4 == 0)) {
> Support64BitDma = FALSE;
> }
>
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> index 1bb701a..68d8a5c 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> @@ -2,7 +2,7 @@
>
> Provides some data structure definitions used by the SD/MMC host
> controller driver.
>
> -Copyright (c) 2018, NVIDIA CORPORATION. All rights reserved.
> +Copyright (c) 2018-2019, NVIDIA CORPORATION. All rights reserved.
> Copyright (c) 2015, Intel Corporation. All rights reserved.<BR>
> This program and the accompanying materials
> are licensed and made available under the terms and conditions of the BSD
> License
> @@ -145,13 +145,15 @@ typedef struct {
> EFI_PHYSICAL_ADDRESS DataPhy;
> VOID *DataMap;
> SD_MMC_HC_TRANSFER_MODE Mode;
> + SD_MMC_HC_ADMA_LEGTH Length;
Maybe:
SD_MMC_HC_ADMA_LENGTH_MODE AdmaLengthMode;
is better to avoid confusion.
>
> EFI_EVENT Event;
> BOOLEAN Started;
> UINT64 Timeout;
>
> SD_MMC_HC_ADMA_32_DESC_LINE *Adma32Desc;
> - SD_MMC_HC_ADMA_64_DESC_LINE *Adma64Desc;
> + SD_MMC_HC_ADMA_64_V3_DESC_LINE *Adma64V3Desc;
> + SD_MMC_HC_ADMA_64_V4_DESC_LINE *Adma64V4Desc;
> EFI_PHYSICAL_ADDRESS AdmaDescPhy;
> VOID *AdmaMap;
> UINT32 AdmaPages;
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> index d73fa10..a6d2395 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> @@ -6,7 +6,7 @@
>
> It would expose EFI_SD_MMC_PASS_THRU_PROTOCOL for upper layer use.
>
> - Copyright (c) 2018, NVIDIA CORPORATION. All rights reserved.
> + Copyright (c) 2018-2019, NVIDIA CORPORATION. All rights reserved.
> Copyright (c) 2015 - 2019, Intel Corporation. All rights reserved.<BR>
> This program and the accompanying materials
> are licensed and made available under the terms and conditions of the BSD
> License
> @@ -1010,18 +1010,32 @@ SdMmcHcInitV4Enhancements (
> if (ControllerVer >= SD_MMC_HC_CTRL_VER_400) {
> HostCtrl2 = SD_MMC_HC_V4_EN;
> //
> - // Check if V4 64bit support is available
> + // Check if controller version V4.0
> //
> - if (Capability.SysBus64V4 != 0) {
> - HostCtrl2 |= SD_MMC_HC_64_ADDR_EN;
> - DEBUG ((DEBUG_INFO, "Enabled V4 64 bit system bus support\n"));
> + if (ControllerVer == SD_MMC_HC_CTRL_VER_400) {
> + //
> + // Check if 64bit support is available
> + //
> + if (Capability.SysBus64V3 != 0) {
> + HostCtrl2 |= SD_MMC_HC_64_ADDR_EN;
> + DEBUG ((DEBUG_INFO, "Enabled V4 64 bit system bus support\n"));
> + }
> }
> //
> // Check if controller version V4.10 or higher
> //
> - if (ControllerVer >= SD_MMC_HC_CTRL_VER_410) {
> - HostCtrl2 |= SD_MMC_HC_26_DATA_LEN_ADMA_EN;
> - DEBUG ((DEBUG_INFO, "Enabled V4 26 bit data length ADMA
> support\n"));
> + else if (ControllerVer >= SD_MMC_HC_CTRL_VER_410) {
> + //
> + // Check if 64bit support is available
> + //
> + if (Capability.SysBus64V4 != 0) {
> + HostCtrl2 |= SD_MMC_HC_64_ADDR_EN;
> + DEBUG ((DEBUG_INFO, "Enabled V4 64 bit system bus support\n"));
> + }
> + if (ControllerVer >= SD_MMC_HC_CTRL_VER_410) {
I think the above 'if' statement can be removed.
> + HostCtrl2 |= SD_MMC_HC_26_DATA_LEN_ADMA_EN;
> + DEBUG ((DEBUG_INFO, "Enabled V4 26 bit data length ADMA
> support\n"));
> + }
> }
> Status = SdMmcHcOrMmio (PciIo, Slot, SD_MMC_HC_HOST_CTRL2, sizeof
> (HostCtrl2), &HostCtrl2);
> if (EFI_ERROR (Status)) {
> @@ -1393,14 +1407,10 @@ BuildAdmaDescTable (
> EFI_PCI_IO_PROTOCOL *PciIo;
> EFI_STATUS Status;
> UINTN Bytes;
> - BOOLEAN AddressingMode64;
> - BOOLEAN DataLength26;
> UINT32 AdmaMaxDataPerLine;
> UINT32 DescSize;
> VOID *AdmaDesc;
>
> - AddressingMode64 = FALSE;
> - DataLength26 = FALSE;
> AdmaMaxDataPerLine = ADMA_MAX_DATA_PER_LINE_16B;
> DescSize = sizeof (SD_MMC_HC_ADMA_32_DESC_LINE);
> AdmaDesc = NULL;
> @@ -1410,27 +1420,16 @@ BuildAdmaDescTable (
> PciIo = Trb->Private->PciIo;
>
> //
> - // Detect whether 64bit addressing is supported.
> - //
> - if (ControllerVer >= SD_MMC_HC_CTRL_VER_400) {
> - Status = SdMmcHcCheckMmioSet(PciIo, Trb->Slot,
> SD_MMC_HC_HOST_CTRL2, sizeof(UINT16),
> - SD_MMC_HC_V4_EN|SD_MMC_HC_64_ADDR_EN,
> SD_MMC_HC_V4_EN|SD_MMC_HC_64_ADDR_EN);
> - if (!EFI_ERROR (Status)) {
> - AddressingMode64 = TRUE;
> - DescSize = sizeof (SD_MMC_HC_ADMA_64_DESC_LINE);
> - }
> - }
> - //
> // Check for valid ranges in 32bit ADMA Descriptor Table
> //
> - if (!AddressingMode64 &&
> + if ((Trb->Mode == SdMmcAdma32bMode) &&
> ((Data >= 0x100000000ul) || ((Data + DataLen) > 0x100000000ul))) {
> return EFI_INVALID_PARAMETER;
> }
> //
> // Check address field alignment
> //
> - if (AddressingMode64) {
> + if (Trb->Mode != SdMmcAdma32bMode) {
> //
> // Address field shall be set on 64-bit boundary (Lower 3-bit is always set
> to 0)
> //
> @@ -1445,13 +1444,19 @@ BuildAdmaDescTable (
> DEBUG ((DEBUG_INFO, "The buffer [0x%x] to construct ADMA desc is not
> aligned to 4 bytes boundary!\n", Data));
> }
> }
> +
> //
> - // Detect whether 26bit data length is supported.
> + // Configure 64b ADMA.
> //
> - Status = SdMmcHcCheckMmioSet(PciIo, Trb->Slot,
> SD_MMC_HC_HOST_CTRL2, sizeof(UINT16),
> - SD_MMC_HC_26_DATA_LEN_ADMA_EN,
> SD_MMC_HC_26_DATA_LEN_ADMA_EN);
> - if (!EFI_ERROR (Status)) {
> - DataLength26 = TRUE;
> + if (Trb->Mode == SdMmcAdma64bV3Mode) {
> + DescSize = sizeof (SD_MMC_HC_ADMA_64_V3_DESC_LINE);
> + }else if (Trb->Mode == SdMmcAdma64bV4Mode) {
> + DescSize = sizeof (SD_MMC_HC_ADMA_64_V4_DESC_LINE);
> + }
> + //
> + // Configure 26b data length.
> + //
> + if (Trb->Length == SdMmcAdmaLen26b) {
> AdmaMaxDataPerLine = ADMA_MAX_DATA_PER_LINE_26B;
> }
>
> @@ -1492,7 +1497,7 @@ BuildAdmaDescTable (
> return EFI_OUT_OF_RESOURCES;
> }
>
> - if ((!AddressingMode64) &&
> + if ((Trb->Mode == SdMmcAdma32bMode) &&
> (UINT64)(UINTN)Trb->AdmaDescPhy > 0x100000000ul) {
> //
> // The ADMA doesn't support 64bit addressing.
> @@ -1511,19 +1516,26 @@ BuildAdmaDescTable (
>
> Remaining = DataLen;
> Address = Data;
> - if (!AddressingMode64) {
> + if (Trb->Mode == SdMmcAdma32bMode) {
> Trb->Adma32Desc = AdmaDesc;
> - Trb->Adma64Desc = NULL;
> + Trb->Adma64V3Desc = NULL;
> + Trb->Adma64V4Desc = NULL;
> + } else if (Trb->Mode == SdMmcAdma64bV3Mode) {
> + Trb->Adma64V3Desc = AdmaDesc;
> + Trb->Adma32Desc = NULL;
> + Trb->Adma64V4Desc = NULL;
> } else {
> - Trb->Adma64Desc = AdmaDesc;
> + Trb->Adma64V4Desc = AdmaDesc;
> Trb->Adma32Desc = NULL;
> + Trb->Adma64V3Desc = NULL;
> }
Suggest to remove those NULL assignments to:
Trb->Adma32Desc
Trb->Adma64Desc
Trb->Adma64V3Desc
Since in SdMmcCreateTrb(), 'Trb' is allocated by:
Trb = AllocateZeroPool (sizeof (SD_MMC_HC_TRB));
> +
> for (Index = 0; Index < Entries; Index++) {
> - if (!AddressingMode64) {
> + if (Trb->Mode == SdMmcAdma32bMode) {
> if (Remaining <= AdmaMaxDataPerLine) {
> Trb->Adma32Desc[Index].Valid = 1;
> Trb->Adma32Desc[Index].Act = 2;
> - if (DataLength26) {
> + if (Trb->Length == SdMmcAdmaLen26b) {
> Trb->Adma32Desc[Index].UpperLength = (UINT16)RShiftU64
> (Remaining, 16);
> }
> Trb->Adma32Desc[Index].LowerLength = (UINT16)(Remaining &
> MAX_UINT16);
> @@ -1532,32 +1544,53 @@ BuildAdmaDescTable (
> } else {
> Trb->Adma32Desc[Index].Valid = 1;
> Trb->Adma32Desc[Index].Act = 2;
> - if (DataLength26) {
> + if (Trb->Length == SdMmcAdmaLen26b) {
> Trb->Adma32Desc[Index].UpperLength = 0;
> }
> Trb->Adma32Desc[Index].LowerLength = 0;
> Trb->Adma32Desc[Index].Address = (UINT32)Address;
> }
> + } else if (Trb->Mode == SdMmcAdma64bV3Mode) {
> + if (Remaining <= AdmaMaxDataPerLine) {
> + Trb->Adma64V3Desc[Index].Valid = 1;
> + Trb->Adma64V3Desc[Index].Act = 2;
> + if (Trb->Length == SdMmcAdmaLen26b) {
> + Trb->Adma64V3Desc[Index].UpperLength = (UINT16)RShiftU64
> (Remaining, 16);
> + }
> + Trb->Adma64V3Desc[Index].LowerLength = (UINT16)(Remaining &
> MAX_UINT16);
> + Trb->Adma64V3Desc[Index].LowerAddress = (UINT32)Address;
> + Trb->Adma64V3Desc[Index].UpperAddress = (UINT32)RShiftU64
> (Address, 32);
> + break;
> + } else {
> + Trb->Adma64V3Desc[Index].Valid = 1;
> + Trb->Adma64V3Desc[Index].Act = 2;
> + if (Trb->Length == SdMmcAdmaLen26b) {
> + Trb->Adma64V3Desc[Index].UpperLength = 0;
> + }
> + Trb->Adma64V3Desc[Index].LowerLength = 0;
> + Trb->Adma64V3Desc[Index].LowerAddress = (UINT32)Address;
> + Trb->Adma64V3Desc[Index].UpperAddress = (UINT32)RShiftU64
> (Address, 32);
> + }
> } else {
> if (Remaining <= AdmaMaxDataPerLine) {
> - Trb->Adma64Desc[Index].Valid = 1;
> - Trb->Adma64Desc[Index].Act = 2;
> - if (DataLength26) {
> - Trb->Adma64Desc[Index].UpperLength = (UINT16)RShiftU64
> (Remaining, 16);
> + Trb->Adma64V4Desc[Index].Valid = 1;
> + Trb->Adma64V4Desc[Index].Act = 2;
> + if (Trb->Length == SdMmcAdmaLen26b) {
> + Trb->Adma64V4Desc[Index].UpperLength = (UINT16)RShiftU64
> (Remaining, 16);
> }
> - Trb->Adma64Desc[Index].LowerLength = (UINT16)(Remaining &
> MAX_UINT16);
> - Trb->Adma64Desc[Index].LowerAddress = (UINT32)Address;
> - Trb->Adma64Desc[Index].UpperAddress = (UINT32)RShiftU64 (Address,
> 32);
> + Trb->Adma64V4Desc[Index].LowerLength = (UINT16)(Remaining &
> MAX_UINT16);
> + Trb->Adma64V4Desc[Index].LowerAddress = (UINT32)Address;
> + Trb->Adma64V4Desc[Index].UpperAddress = (UINT32)RShiftU64
> (Address, 32);
> break;
> } else {
> - Trb->Adma64Desc[Index].Valid = 1;
> - Trb->Adma64Desc[Index].Act = 2;
> - if (DataLength26) {
> - Trb->Adma64Desc[Index].UpperLength = 0;
> + Trb->Adma64V4Desc[Index].Valid = 1;
> + Trb->Adma64V4Desc[Index].Act = 2;
> + if (Trb->Length == SdMmcAdmaLen26b) {
> + Trb->Adma64V4Desc[Index].UpperLength = 0;
> }
> - Trb->Adma64Desc[Index].LowerLength = 0;
> - Trb->Adma64Desc[Index].LowerAddress = (UINT32)Address;
> - Trb->Adma64Desc[Index].UpperAddress = (UINT32)RShiftU64 (Address,
> 32);
> + Trb->Adma64V4Desc[Index].LowerLength = 0;
> + Trb->Adma64V4Desc[Index].LowerAddress = (UINT32)Address;
> + Trb->Adma64V4Desc[Index].UpperAddress = (UINT32)RShiftU64
> (Address, 32);
> }
> }
>
> @@ -1568,7 +1601,13 @@ BuildAdmaDescTable (
> //
> // Set the last descriptor line as end of descriptor table
> //
> - AddressingMode64 ? (Trb->Adma64Desc[Index].End = 1) : (Trb-
> >Adma32Desc[Index].End = 1);
> + if (Trb->Mode == SdMmcAdma32bMode) {
> + Trb->Adma32Desc[Index].End = 1;
> + } else if (Trb->Mode == SdMmcAdma64bV3Mode) {
> + Trb->Adma64V3Desc[Index].End = 1;
> + } else {
> + Trb->Adma64V4Desc[Index].End = 1;
> + }
> return EFI_SUCCESS;
> }
>
> @@ -1665,7 +1704,20 @@ SdMmcCreateTrb (
> if (Trb->DataLen == 0) {
> Trb->Mode = SdMmcNoData;
> } else if (Private->Capability[Slot].Adma2 != 0) {
> - Trb->Mode = SdMmcAdmaMode;
> + Trb->Mode = SdMmcAdma32bMode;
> + Trb->Length = SdMmcAdmaLen16b;
> + if ((Private->ControllerVersion[Slot] == SD_MMC_HC_CTRL_VER_300)
> &&
> + (Private->Capability[Slot].SysBus64V3 == 1)) {
> + Trb->Mode = SdMmcAdma64bV3Mode;
> + } else if (((Private->ControllerVersion[Slot] ==
> SD_MMC_HC_CTRL_VER_400) &&
> + (Private->Capability[Slot].SysBus64V3 == 1)) ||
> + ((Private->ControllerVersion[Slot] >= SD_MMC_HC_CTRL_VER_410)
> &&
> + (Private->Capability[Slot].SysBus64V4 == 1))) {
> + Trb->Mode = SdMmcAdma64bV4Mode;
> + }
> + if (Private->ControllerVersion[Slot] >= SD_MMC_HC_CTRL_VER_410) {
> + Trb->Length = SdMmcAdmaLen26b;
> + }
> Status = BuildAdmaDescTable (Trb, Private->ControllerVersion[Slot]);
> if (EFI_ERROR (Status)) {
> PciIo->Unmap (PciIo, Trb->DataMap);
> @@ -1719,11 +1771,18 @@ SdMmcFreeTrb (
> Trb->Adma32Desc
> );
> }
> - if (Trb->Adma64Desc != NULL) {
> + if (Trb->Adma64V3Desc != NULL) {
> + PciIo->FreeBuffer (
> + PciIo,
> + Trb->AdmaPages,
> + Trb->Adma64V3Desc
> + );
> + }
> + if (Trb->Adma64V4Desc != NULL) {
> PciIo->FreeBuffer (
> PciIo,
> Trb->AdmaPages,
> - Trb->Adma64Desc
> + Trb->Adma64V4Desc
> );
> }
> if (Trb->DataMap != NULL) {
> @@ -1891,27 +1950,35 @@ SdMmcExecTrb (
> if (EFI_ERROR (Status)) {
> return Status;
> }
> +
> + if (Private->ControllerVersion[Trb->Slot] >= SD_MMC_HC_CTRL_VER_400)
> {
> + Status = SdMmcHcCheckMmioSet(PciIo, Trb->Slot,
> SD_MMC_HC_HOST_CTRL2, sizeof(UINT16),
> + SD_MMC_HC_64_ADDR_EN, SD_MMC_HC_64_ADDR_EN);
> + if (!EFI_ERROR (Status)) {
> + AddressingMode64 = TRUE;
> + }
> + }
> +
> //
> // Set Host Control 1 register DMA Select field
> //
> - if (Trb->Mode == SdMmcAdmaMode) {
> + if ((Trb->Mode == SdMmcAdma32bMode) ||
> + (Trb->Mode == SdMmcAdma64bV4Mode)) {
> HostCtrl1 = BIT4;
> Status = SdMmcHcOrMmio (PciIo, Trb->Slot, SD_MMC_HC_HOST_CTRL1,
> sizeof (HostCtrl1), &HostCtrl1);
> if (EFI_ERROR (Status)) {
> return Status;
> }
> + } else if (Trb->Mode == SdMmcAdma64bV3Mode) {
> + HostCtrl1 = BIT4|BIT3;
> + Status = SdMmcHcOrMmio (PciIo, Trb->Slot, SD_MMC_HC_HOST_CTRL1,
> sizeof (HostCtrl1), &HostCtrl1);
> + if (EFI_ERROR (Status)) {
> + return Status;
> + }
> }
>
> SdMmcHcLedOnOff (PciIo, Trb->Slot, TRUE);
>
> - if (Private->ControllerVersion[Trb->Slot] >= SD_MMC_HC_CTRL_VER_400)
> {
> - Status = SdMmcHcCheckMmioSet(PciIo, Trb->Slot,
> SD_MMC_HC_HOST_CTRL2, sizeof(UINT16),
> - SD_MMC_HC_V4_EN|SD_MMC_HC_64_ADDR_EN,
> SD_MMC_HC_V4_EN|SD_MMC_HC_64_ADDR_EN);
> - if (!EFI_ERROR (Status)) {
> - AddressingMode64 = TRUE;
> - }
> - }
> -
> if (Trb->Mode == SdMmcSdmaMode) {
> if ((!AddressingMode64) &&
> ((UINT64)(UINTN)Trb->DataPhy >= 0x100000000ul)) {
> @@ -1929,7 +1996,9 @@ SdMmcExecTrb (
> if (EFI_ERROR (Status)) {
> return Status;
> }
> - } else if (Trb->Mode == SdMmcAdmaMode) {
> + } else if ((Trb->Mode == SdMmcAdma32bMode) ||
> + (Trb->Mode == SdMmcAdma64bV3Mode) ||
> + (Trb->Mode == SdMmcAdma64bV4Mode)) {
> AdmaAddr = (UINT64)(UINTN)Trb->AdmaDescPhy;
> Status = SdMmcHcRwMmio (PciIo, Trb->Slot,
> SD_MMC_HC_ADMA_SYS_ADDR, FALSE, sizeof (AdmaAddr), &AdmaAddr);
> if (EFI_ERROR (Status)) {
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> index d157f2c..3a05456 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> @@ -2,7 +2,7 @@
>
> Provides some data structure definitions used by the SD/MMC host
> controller driver.
>
> -Copyright (c) 2018, NVIDIA CORPORATION. All rights reserved.
> +Copyright (c) 2018-2019, NVIDIA CORPORATION. All rights reserved.
> Copyright (c) 2015, Intel Corporation. All rights reserved.<BR>
> This program and the accompanying materials
> are licensed and made available under the terms and conditions of the BSD
> License
> @@ -80,16 +80,25 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF
> ANY KIND, EITHER EXPRESS OR IMPLIED.
>
> //
> // The transfer modes supported by SD Host Controller
> -// Simplified Spec 3.0 Table 1-2
> //
> typedef enum {
> SdMmcNoData,
> SdMmcPioMode,
> SdMmcSdmaMode,
> - SdMmcAdmaMode
> + SdMmcAdma32bMode,
> + SdMmcAdma64bV3Mode,
> + SdMmcAdma64bV4Mode
> } SD_MMC_HC_TRANSFER_MODE;
>
> //
> +// The ADMA transfer lengths supported by SD Host Controller
> +//
> +typedef enum {
> + SdMmcAdmaLen16b,
> + SdMmcAdmaLen26b
> +} SD_MMC_HC_ADMA_LEGTH;
Typo 'LEGTH' -> 'LENGTH'
Also, how about 'SD_MMC_HC_ADMA_LENGTH_MODE'?
Best Regards,
Hao Wu
> +
> +//
> // The maximum data length of each descriptor line
> //
> #define ADMA_MAX_DATA_PER_LINE_16B SIZE_64KB
> @@ -122,8 +131,20 @@ typedef struct {
> UINT32 LowerLength:16;
> UINT32 LowerAddress;
> UINT32 UpperAddress;
> +} SD_MMC_HC_ADMA_64_V3_DESC_LINE;
> +
> +typedef struct {
> + UINT32 Valid:1;
> + UINT32 End:1;
> + UINT32 Int:1;
> + UINT32 Reserved:1;
> + UINT32 Act:2;
> + UINT32 UpperLength:10;
> + UINT32 LowerLength:16;
> + UINT32 LowerAddress;
> + UINT32 UpperAddress;
> UINT32 Reserved1;
> -} SD_MMC_HC_ADMA_64_DESC_LINE;
> +} SD_MMC_HC_ADMA_64_V4_DESC_LINE;
>
> #define SD_MMC_SDMA_BOUNDARY 512 * 1024
> #define SD_MMC_SDMA_ROUND_UP(x, n) (((x) + n) & ~(n - 1))
> --
> 2.7.4
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] MdeModulePkg/SdMmcPciHcDxe: Add V3 64b DMA Support
2019-03-06 3:01 ` Wu, Hao A
@ 2019-03-06 5:17 ` Ashish Singhal
2019-03-06 23:05 ` Cohen, Eugene
1 sibling, 0 replies; 9+ messages in thread
From: Ashish Singhal @ 2019-03-06 5:17 UTC (permalink / raw)
To: Wu, Hao A, edk2-devel@lists.01.org
Hi Hao,
That is right. Also, I have addressed your comments and have submitted v2 patch.
Thanks
Ashish
-----Original Message-----
From: Wu, Hao A <hao.a.wu@intel.com>
Sent: Tuesday, March 5, 2019 8:01 PM
To: Ashish Singhal <ashishsingha@nvidia.com>; edk2-devel@lists.01.org
Cc: eugene@hp.com
Subject: RE: [PATCH] MdeModulePkg/SdMmcPciHcDxe: Add V3 64b DMA Support
Hi Ashish,
One thing to confirm, for the updated checks within
SdMmcPciHcDriverBindingStart():
> if ((Private->ControllerVersion[Slot] == SD_MMC_HC_CTRL_VER_300 &&
> Private->Capability[Slot].SysBus64V3 == 0) ||
> (Private->ControllerVersion[Slot] == SD_MMC_HC_CTRL_VER_400 &&
> Private->Capability[Slot].SysBus64V3 == 0) ||
> (Private->ControllerVersion[Slot] >= SD_MMC_HC_CTRL_VER_410 &&
> Private->Capability[Slot].SysBus64V4 == 0)) {
> Support64BitDma = FALSE;
> }
When the SDHC with version greater than 4.10, the check is only performed against the 'SysBus64V4' bit. My understanding of the purpose is that:
1. For SDHC with version 4.00, the support of V3 mode and V4 mode of 64-bit System Address are reflect by bit 'SysBus64V3'. Thus, I can infer that the possible support case is both or neither.
2. The spec states that SDHC with version greater than 4.10 divides the V3 mode and V4 mode support into 2 bits (SysBus64V3, SysBus64V4) so that the
V3 mode support can be optional.
So based on 1 & 2, we do not even bother to check the 'SysBus64V3' bit when HC version >= 4.10. Is that right?
I verified the patch on SDHC version 3.00 with 64-bit System Address Support. Hope more configurations are available for testing on Eugene's side.
Besides, some minor comments below:
> -----Original Message-----
> From: Ashish Singhal [mailto:ashishsingha@nvidia.com]
> Sent: Saturday, March 02, 2019 2:30 AM
> To: edk2-devel@lists.01.org
> Cc: Wu, Hao A; eugene@hp.com; Ashish Singhal
> Subject: [PATCH] MdeModulePkg/SdMmcPciHcDxe: Add V3 64b DMA Support
Please help to add the below Bugzilla tracker for reference:
https://bugzilla.tianocore.org/show_bug.cgi?id=1583
I have updated the above tracker to match the purpose of the proposed patch.
>
> Driver was supporting only 32b DMA support for V3 controllers. Add
> support for 64b DMA as well for completeness.
>
> For V4.0 64b support, driver was looking at incorrect capability
> register bit. Fix for that is present as well.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ashish Singhal <ashishsingha@nvidia.com>
> ---
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c | 10 +-
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h | 6 +-
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c | 199
> ++++++++++++++-------
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h | 29 ++-
> 4 files changed, 170 insertions(+), 74 deletions(-)
>
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> index b474f8d..9b7b88c 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> @@ -6,7 +6,7 @@
>
> It would expose EFI_SD_MMC_PASS_THRU_PROTOCOL for upper layer use.
>
> - Copyright (c) 2018, NVIDIA CORPORATION. All rights reserved.
> + Copyright (c) 2018-2019, NVIDIA CORPORATION. All rights reserved.
> Copyright (c) 2015 - 2019, Intel Corporation. All rights reserved.<BR>
> This program and the accompanying materials
> are licensed and made available under the terms and conditions of
> the BSD License @@ -666,8 +666,12 @@ SdMmcPciHcDriverBindingStart (
> // If any of the slots does not support 64b system bus
> // do not enable 64b DMA in the PCI layer.
> //
> - if (Private->Capability[Slot].SysBus64V3 == 0 &&
> - Private->Capability[Slot].SysBus64V4 == 0) {
> + if ((Private->ControllerVersion[Slot] == SD_MMC_HC_CTRL_VER_300 &&
> + Private->Capability[Slot].SysBus64V3 == 0) ||
> + (Private->ControllerVersion[Slot] == SD_MMC_HC_CTRL_VER_400 &&
> + Private->Capability[Slot].SysBus64V3 == 0) ||
> + (Private->ControllerVersion[Slot] >= SD_MMC_HC_CTRL_VER_410 &&
> + Private->Capability[Slot].SysBus64V4 == 0)) {
> Support64BitDma = FALSE;
> }
>
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> index 1bb701a..68d8a5c 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> @@ -2,7 +2,7 @@
>
> Provides some data structure definitions used by the SD/MMC host
> controller driver.
>
> -Copyright (c) 2018, NVIDIA CORPORATION. All rights reserved.
> +Copyright (c) 2018-2019, NVIDIA CORPORATION. All rights reserved.
> Copyright (c) 2015, Intel Corporation. All rights reserved.<BR> This
> program and the accompanying materials are licensed and made
> available under the terms and conditions of the BSD License @@ -145,13
> +145,15 @@ typedef struct {
> EFI_PHYSICAL_ADDRESS DataPhy;
> VOID *DataMap;
> SD_MMC_HC_TRANSFER_MODE Mode;
> + SD_MMC_HC_ADMA_LEGTH Length;
Maybe:
SD_MMC_HC_ADMA_LENGTH_MODE AdmaLengthMode;
is better to avoid confusion.
>
> EFI_EVENT Event;
> BOOLEAN Started;
> UINT64 Timeout;
>
> SD_MMC_HC_ADMA_32_DESC_LINE *Adma32Desc;
> - SD_MMC_HC_ADMA_64_DESC_LINE *Adma64Desc;
> + SD_MMC_HC_ADMA_64_V3_DESC_LINE *Adma64V3Desc;
> + SD_MMC_HC_ADMA_64_V4_DESC_LINE *Adma64V4Desc;
> EFI_PHYSICAL_ADDRESS AdmaDescPhy;
> VOID *AdmaMap;
> UINT32 AdmaPages;
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> index d73fa10..a6d2395 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> @@ -6,7 +6,7 @@
>
> It would expose EFI_SD_MMC_PASS_THRU_PROTOCOL for upper layer use.
>
> - Copyright (c) 2018, NVIDIA CORPORATION. All rights reserved.
> + Copyright (c) 2018-2019, NVIDIA CORPORATION. All rights reserved.
> Copyright (c) 2015 - 2019, Intel Corporation. All rights reserved.<BR>
> This program and the accompanying materials
> are licensed and made available under the terms and conditions of
> the BSD License @@ -1010,18 +1010,32 @@ SdMmcHcInitV4Enhancements (
> if (ControllerVer >= SD_MMC_HC_CTRL_VER_400) {
> HostCtrl2 = SD_MMC_HC_V4_EN;
> //
> - // Check if V4 64bit support is available
> + // Check if controller version V4.0
> //
> - if (Capability.SysBus64V4 != 0) {
> - HostCtrl2 |= SD_MMC_HC_64_ADDR_EN;
> - DEBUG ((DEBUG_INFO, "Enabled V4 64 bit system bus support\n"));
> + if (ControllerVer == SD_MMC_HC_CTRL_VER_400) {
> + //
> + // Check if 64bit support is available
> + //
> + if (Capability.SysBus64V3 != 0) {
> + HostCtrl2 |= SD_MMC_HC_64_ADDR_EN;
> + DEBUG ((DEBUG_INFO, "Enabled V4 64 bit system bus support\n"));
> + }
> }
> //
> // Check if controller version V4.10 or higher
> //
> - if (ControllerVer >= SD_MMC_HC_CTRL_VER_410) {
> - HostCtrl2 |= SD_MMC_HC_26_DATA_LEN_ADMA_EN;
> - DEBUG ((DEBUG_INFO, "Enabled V4 26 bit data length ADMA
> support\n"));
> + else if (ControllerVer >= SD_MMC_HC_CTRL_VER_410) {
> + //
> + // Check if 64bit support is available
> + //
> + if (Capability.SysBus64V4 != 0) {
> + HostCtrl2 |= SD_MMC_HC_64_ADDR_EN;
> + DEBUG ((DEBUG_INFO, "Enabled V4 64 bit system bus support\n"));
> + }
> + if (ControllerVer >= SD_MMC_HC_CTRL_VER_410) {
I think the above 'if' statement can be removed.
> + HostCtrl2 |= SD_MMC_HC_26_DATA_LEN_ADMA_EN;
> + DEBUG ((DEBUG_INFO, "Enabled V4 26 bit data length ADMA
> support\n"));
> + }
> }
> Status = SdMmcHcOrMmio (PciIo, Slot, SD_MMC_HC_HOST_CTRL2, sizeof
> (HostCtrl2), &HostCtrl2);
> if (EFI_ERROR (Status)) {
> @@ -1393,14 +1407,10 @@ BuildAdmaDescTable (
> EFI_PCI_IO_PROTOCOL *PciIo;
> EFI_STATUS Status;
> UINTN Bytes;
> - BOOLEAN AddressingMode64;
> - BOOLEAN DataLength26;
> UINT32 AdmaMaxDataPerLine;
> UINT32 DescSize;
> VOID *AdmaDesc;
>
> - AddressingMode64 = FALSE;
> - DataLength26 = FALSE;
> AdmaMaxDataPerLine = ADMA_MAX_DATA_PER_LINE_16B;
> DescSize = sizeof (SD_MMC_HC_ADMA_32_DESC_LINE);
> AdmaDesc = NULL;
> @@ -1410,27 +1420,16 @@ BuildAdmaDescTable (
> PciIo = Trb->Private->PciIo;
>
> //
> - // Detect whether 64bit addressing is supported.
> - //
> - if (ControllerVer >= SD_MMC_HC_CTRL_VER_400) {
> - Status = SdMmcHcCheckMmioSet(PciIo, Trb->Slot,
> SD_MMC_HC_HOST_CTRL2, sizeof(UINT16),
> - SD_MMC_HC_V4_EN|SD_MMC_HC_64_ADDR_EN,
> SD_MMC_HC_V4_EN|SD_MMC_HC_64_ADDR_EN);
> - if (!EFI_ERROR (Status)) {
> - AddressingMode64 = TRUE;
> - DescSize = sizeof (SD_MMC_HC_ADMA_64_DESC_LINE);
> - }
> - }
> - //
> // Check for valid ranges in 32bit ADMA Descriptor Table
> //
> - if (!AddressingMode64 &&
> + if ((Trb->Mode == SdMmcAdma32bMode) &&
> ((Data >= 0x100000000ul) || ((Data + DataLen) > 0x100000000ul))) {
> return EFI_INVALID_PARAMETER;
> }
> //
> // Check address field alignment
> //
> - if (AddressingMode64) {
> + if (Trb->Mode != SdMmcAdma32bMode) {
> //
> // Address field shall be set on 64-bit boundary (Lower 3-bit is
> always set to 0)
> //
> @@ -1445,13 +1444,19 @@ BuildAdmaDescTable (
> DEBUG ((DEBUG_INFO, "The buffer [0x%x] to construct ADMA desc
> is not aligned to 4 bytes boundary!\n", Data));
> }
> }
> +
> //
> - // Detect whether 26bit data length is supported.
> + // Configure 64b ADMA.
> //
> - Status = SdMmcHcCheckMmioSet(PciIo, Trb->Slot,
> SD_MMC_HC_HOST_CTRL2, sizeof(UINT16),
> - SD_MMC_HC_26_DATA_LEN_ADMA_EN,
> SD_MMC_HC_26_DATA_LEN_ADMA_EN);
> - if (!EFI_ERROR (Status)) {
> - DataLength26 = TRUE;
> + if (Trb->Mode == SdMmcAdma64bV3Mode) {
> + DescSize = sizeof (SD_MMC_HC_ADMA_64_V3_DESC_LINE); }else if
> + (Trb->Mode == SdMmcAdma64bV4Mode) {
> + DescSize = sizeof (SD_MMC_HC_ADMA_64_V4_DESC_LINE); } // //
> + Configure 26b data length.
> + //
> + if (Trb->Length == SdMmcAdmaLen26b) {
> AdmaMaxDataPerLine = ADMA_MAX_DATA_PER_LINE_26B;
> }
>
> @@ -1492,7 +1497,7 @@ BuildAdmaDescTable (
> return EFI_OUT_OF_RESOURCES;
> }
>
> - if ((!AddressingMode64) &&
> + if ((Trb->Mode == SdMmcAdma32bMode) &&
> (UINT64)(UINTN)Trb->AdmaDescPhy > 0x100000000ul) {
> //
> // The ADMA doesn't support 64bit addressing.
> @@ -1511,19 +1516,26 @@ BuildAdmaDescTable (
>
> Remaining = DataLen;
> Address = Data;
> - if (!AddressingMode64) {
> + if (Trb->Mode == SdMmcAdma32bMode) {
> Trb->Adma32Desc = AdmaDesc;
> - Trb->Adma64Desc = NULL;
> + Trb->Adma64V3Desc = NULL;
> + Trb->Adma64V4Desc = NULL;
> + } else if (Trb->Mode == SdMmcAdma64bV3Mode) {
> + Trb->Adma64V3Desc = AdmaDesc;
> + Trb->Adma32Desc = NULL;
> + Trb->Adma64V4Desc = NULL;
> } else {
> - Trb->Adma64Desc = AdmaDesc;
> + Trb->Adma64V4Desc = AdmaDesc;
> Trb->Adma32Desc = NULL;
> + Trb->Adma64V3Desc = NULL;
> }
Suggest to remove those NULL assignments to:
Trb->Adma32Desc
Trb->Adma64Desc
Trb->Adma64V3Desc
Since in SdMmcCreateTrb(), 'Trb' is allocated by:
Trb = AllocateZeroPool (sizeof (SD_MMC_HC_TRB));
> +
> for (Index = 0; Index < Entries; Index++) {
> - if (!AddressingMode64) {
> + if (Trb->Mode == SdMmcAdma32bMode) {
> if (Remaining <= AdmaMaxDataPerLine) {
> Trb->Adma32Desc[Index].Valid = 1;
> Trb->Adma32Desc[Index].Act = 2;
> - if (DataLength26) {
> + if (Trb->Length == SdMmcAdmaLen26b) {
> Trb->Adma32Desc[Index].UpperLength = (UINT16)RShiftU64
> (Remaining, 16);
> }
> Trb->Adma32Desc[Index].LowerLength = (UINT16)(Remaining &
> MAX_UINT16); @@ -1532,32 +1544,53 @@ BuildAdmaDescTable (
> } else {
> Trb->Adma32Desc[Index].Valid = 1;
> Trb->Adma32Desc[Index].Act = 2;
> - if (DataLength26) {
> + if (Trb->Length == SdMmcAdmaLen26b) {
> Trb->Adma32Desc[Index].UpperLength = 0;
> }
> Trb->Adma32Desc[Index].LowerLength = 0;
> Trb->Adma32Desc[Index].Address = (UINT32)Address;
> }
> + } else if (Trb->Mode == SdMmcAdma64bV3Mode) {
> + if (Remaining <= AdmaMaxDataPerLine) {
> + Trb->Adma64V3Desc[Index].Valid = 1;
> + Trb->Adma64V3Desc[Index].Act = 2;
> + if (Trb->Length == SdMmcAdmaLen26b) {
> + Trb->Adma64V3Desc[Index].UpperLength = (UINT16)RShiftU64
> (Remaining, 16);
> + }
> + Trb->Adma64V3Desc[Index].LowerLength = (UINT16)(Remaining &
> MAX_UINT16);
> + Trb->Adma64V3Desc[Index].LowerAddress = (UINT32)Address;
> + Trb->Adma64V3Desc[Index].UpperAddress = (UINT32)RShiftU64
> (Address, 32);
> + break;
> + } else {
> + Trb->Adma64V3Desc[Index].Valid = 1;
> + Trb->Adma64V3Desc[Index].Act = 2;
> + if (Trb->Length == SdMmcAdmaLen26b) {
> + Trb->Adma64V3Desc[Index].UpperLength = 0;
> + }
> + Trb->Adma64V3Desc[Index].LowerLength = 0;
> + Trb->Adma64V3Desc[Index].LowerAddress = (UINT32)Address;
> + Trb->Adma64V3Desc[Index].UpperAddress = (UINT32)RShiftU64
> (Address, 32);
> + }
> } else {
> if (Remaining <= AdmaMaxDataPerLine) {
> - Trb->Adma64Desc[Index].Valid = 1;
> - Trb->Adma64Desc[Index].Act = 2;
> - if (DataLength26) {
> - Trb->Adma64Desc[Index].UpperLength = (UINT16)RShiftU64
> (Remaining, 16);
> + Trb->Adma64V4Desc[Index].Valid = 1;
> + Trb->Adma64V4Desc[Index].Act = 2;
> + if (Trb->Length == SdMmcAdmaLen26b) {
> + Trb->Adma64V4Desc[Index].UpperLength = (UINT16)RShiftU64
> (Remaining, 16);
> }
> - Trb->Adma64Desc[Index].LowerLength = (UINT16)(Remaining &
> MAX_UINT16);
> - Trb->Adma64Desc[Index].LowerAddress = (UINT32)Address;
> - Trb->Adma64Desc[Index].UpperAddress = (UINT32)RShiftU64 (Address,
> 32);
> + Trb->Adma64V4Desc[Index].LowerLength = (UINT16)(Remaining &
> MAX_UINT16);
> + Trb->Adma64V4Desc[Index].LowerAddress = (UINT32)Address;
> + Trb->Adma64V4Desc[Index].UpperAddress = (UINT32)RShiftU64
> (Address, 32);
> break;
> } else {
> - Trb->Adma64Desc[Index].Valid = 1;
> - Trb->Adma64Desc[Index].Act = 2;
> - if (DataLength26) {
> - Trb->Adma64Desc[Index].UpperLength = 0;
> + Trb->Adma64V4Desc[Index].Valid = 1;
> + Trb->Adma64V4Desc[Index].Act = 2;
> + if (Trb->Length == SdMmcAdmaLen26b) {
> + Trb->Adma64V4Desc[Index].UpperLength = 0;
> }
> - Trb->Adma64Desc[Index].LowerLength = 0;
> - Trb->Adma64Desc[Index].LowerAddress = (UINT32)Address;
> - Trb->Adma64Desc[Index].UpperAddress = (UINT32)RShiftU64 (Address,
> 32);
> + Trb->Adma64V4Desc[Index].LowerLength = 0;
> + Trb->Adma64V4Desc[Index].LowerAddress = (UINT32)Address;
> + Trb->Adma64V4Desc[Index].UpperAddress = (UINT32)RShiftU64
> (Address, 32);
> }
> }
>
> @@ -1568,7 +1601,13 @@ BuildAdmaDescTable (
> //
> // Set the last descriptor line as end of descriptor table
> //
> - AddressingMode64 ? (Trb->Adma64Desc[Index].End = 1) : (Trb-
> >Adma32Desc[Index].End = 1);
> + if (Trb->Mode == SdMmcAdma32bMode) {
> + Trb->Adma32Desc[Index].End = 1;
> + } else if (Trb->Mode == SdMmcAdma64bV3Mode) {
> + Trb->Adma64V3Desc[Index].End = 1; } else {
> + Trb->Adma64V4Desc[Index].End = 1; }
> return EFI_SUCCESS;
> }
>
> @@ -1665,7 +1704,20 @@ SdMmcCreateTrb (
> if (Trb->DataLen == 0) {
> Trb->Mode = SdMmcNoData;
> } else if (Private->Capability[Slot].Adma2 != 0) {
> - Trb->Mode = SdMmcAdmaMode;
> + Trb->Mode = SdMmcAdma32bMode;
> + Trb->Length = SdMmcAdmaLen16b;
> + if ((Private->ControllerVersion[Slot] ==
> + SD_MMC_HC_CTRL_VER_300)
> &&
> + (Private->Capability[Slot].SysBus64V3 == 1)) {
> + Trb->Mode = SdMmcAdma64bV3Mode;
> + } else if (((Private->ControllerVersion[Slot] ==
> SD_MMC_HC_CTRL_VER_400) &&
> + (Private->Capability[Slot].SysBus64V3 == 1)) ||
> + ((Private->ControllerVersion[Slot] >=
> + SD_MMC_HC_CTRL_VER_410)
> &&
> + (Private->Capability[Slot].SysBus64V4 == 1))) {
> + Trb->Mode = SdMmcAdma64bV4Mode;
> + }
> + if (Private->ControllerVersion[Slot] >= SD_MMC_HC_CTRL_VER_410) {
> + Trb->Length = SdMmcAdmaLen26b;
> + }
> Status = BuildAdmaDescTable (Trb, Private->ControllerVersion[Slot]);
> if (EFI_ERROR (Status)) {
> PciIo->Unmap (PciIo, Trb->DataMap); @@ -1719,11 +1771,18 @@
> SdMmcFreeTrb (
> Trb->Adma32Desc
> );
> }
> - if (Trb->Adma64Desc != NULL) {
> + if (Trb->Adma64V3Desc != NULL) {
> + PciIo->FreeBuffer (
> + PciIo,
> + Trb->AdmaPages,
> + Trb->Adma64V3Desc
> + );
> + }
> + if (Trb->Adma64V4Desc != NULL) {
> PciIo->FreeBuffer (
> PciIo,
> Trb->AdmaPages,
> - Trb->Adma64Desc
> + Trb->Adma64V4Desc
> );
> }
> if (Trb->DataMap != NULL) {
> @@ -1891,27 +1950,35 @@ SdMmcExecTrb (
> if (EFI_ERROR (Status)) {
> return Status;
> }
> +
> + if (Private->ControllerVersion[Trb->Slot] >=
> + SD_MMC_HC_CTRL_VER_400)
> {
> + Status = SdMmcHcCheckMmioSet(PciIo, Trb->Slot,
> SD_MMC_HC_HOST_CTRL2, sizeof(UINT16),
> + SD_MMC_HC_64_ADDR_EN, SD_MMC_HC_64_ADDR_EN);
> + if (!EFI_ERROR (Status)) {
> + AddressingMode64 = TRUE;
> + }
> + }
> +
> //
> // Set Host Control 1 register DMA Select field
> //
> - if (Trb->Mode == SdMmcAdmaMode) {
> + if ((Trb->Mode == SdMmcAdma32bMode) ||
> + (Trb->Mode == SdMmcAdma64bV4Mode)) {
> HostCtrl1 = BIT4;
> Status = SdMmcHcOrMmio (PciIo, Trb->Slot, SD_MMC_HC_HOST_CTRL1,
> sizeof (HostCtrl1), &HostCtrl1);
> if (EFI_ERROR (Status)) {
> return Status;
> }
> + } else if (Trb->Mode == SdMmcAdma64bV3Mode) {
> + HostCtrl1 = BIT4|BIT3;
> + Status = SdMmcHcOrMmio (PciIo, Trb->Slot, SD_MMC_HC_HOST_CTRL1,
> sizeof (HostCtrl1), &HostCtrl1);
> + if (EFI_ERROR (Status)) {
> + return Status;
> + }
> }
>
> SdMmcHcLedOnOff (PciIo, Trb->Slot, TRUE);
>
> - if (Private->ControllerVersion[Trb->Slot] >=
> SD_MMC_HC_CTRL_VER_400) {
> - Status = SdMmcHcCheckMmioSet(PciIo, Trb->Slot,
> SD_MMC_HC_HOST_CTRL2, sizeof(UINT16),
> - SD_MMC_HC_V4_EN|SD_MMC_HC_64_ADDR_EN,
> SD_MMC_HC_V4_EN|SD_MMC_HC_64_ADDR_EN);
> - if (!EFI_ERROR (Status)) {
> - AddressingMode64 = TRUE;
> - }
> - }
> -
> if (Trb->Mode == SdMmcSdmaMode) {
> if ((!AddressingMode64) &&
> ((UINT64)(UINTN)Trb->DataPhy >= 0x100000000ul)) { @@ -1929,7
> +1996,9 @@ SdMmcExecTrb (
> if (EFI_ERROR (Status)) {
> return Status;
> }
> - } else if (Trb->Mode == SdMmcAdmaMode) {
> + } else if ((Trb->Mode == SdMmcAdma32bMode) ||
> + (Trb->Mode == SdMmcAdma64bV3Mode) ||
> + (Trb->Mode == SdMmcAdma64bV4Mode)) {
> AdmaAddr = (UINT64)(UINTN)Trb->AdmaDescPhy;
> Status = SdMmcHcRwMmio (PciIo, Trb->Slot,
> SD_MMC_HC_ADMA_SYS_ADDR, FALSE, sizeof (AdmaAddr), &AdmaAddr);
> if (EFI_ERROR (Status)) {
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> index d157f2c..3a05456 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> @@ -2,7 +2,7 @@
>
> Provides some data structure definitions used by the SD/MMC host
> controller driver.
>
> -Copyright (c) 2018, NVIDIA CORPORATION. All rights reserved.
> +Copyright (c) 2018-2019, NVIDIA CORPORATION. All rights reserved.
> Copyright (c) 2015, Intel Corporation. All rights reserved.<BR> This
> program and the accompanying materials are licensed and made
> available under the terms and conditions of the BSD License @@ -80,16
> +80,25 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
> EXPRESS OR IMPLIED.
>
> //
> // The transfer modes supported by SD Host Controller -// Simplified
> Spec 3.0 Table 1-2 // typedef enum {
> SdMmcNoData,
> SdMmcPioMode,
> SdMmcSdmaMode,
> - SdMmcAdmaMode
> + SdMmcAdma32bMode,
> + SdMmcAdma64bV3Mode,
> + SdMmcAdma64bV4Mode
> } SD_MMC_HC_TRANSFER_MODE;
>
> //
> +// The ADMA transfer lengths supported by SD Host Controller //
> +typedef enum {
> + SdMmcAdmaLen16b,
> + SdMmcAdmaLen26b
> +} SD_MMC_HC_ADMA_LEGTH;
Typo 'LEGTH' -> 'LENGTH'
Also, how about 'SD_MMC_HC_ADMA_LENGTH_MODE'?
Best Regards,
Hao Wu
> +
> +//
> // The maximum data length of each descriptor line //
> #define ADMA_MAX_DATA_PER_LINE_16B SIZE_64KB
> @@ -122,8 +131,20 @@ typedef struct {
> UINT32 LowerLength:16;
> UINT32 LowerAddress;
> UINT32 UpperAddress;
> +} SD_MMC_HC_ADMA_64_V3_DESC_LINE;
> +
> +typedef struct {
> + UINT32 Valid:1;
> + UINT32 End:1;
> + UINT32 Int:1;
> + UINT32 Reserved:1;
> + UINT32 Act:2;
> + UINT32 UpperLength:10;
> + UINT32 LowerLength:16;
> + UINT32 LowerAddress;
> + UINT32 UpperAddress;
> UINT32 Reserved1;
> -} SD_MMC_HC_ADMA_64_DESC_LINE;
> +} SD_MMC_HC_ADMA_64_V4_DESC_LINE;
>
> #define SD_MMC_SDMA_BOUNDARY 512 * 1024
> #define SD_MMC_SDMA_ROUND_UP(x, n) (((x) + n) & ~(n - 1))
> --
> 2.7.4
-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information. Any unauthorized review, use, disclosure or distribution
is prohibited. If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] MdeModulePkg/SdMmcPciHcDxe: Add V3 64b DMA Support
2019-03-06 3:01 ` Wu, Hao A
2019-03-06 5:17 ` Ashish Singhal
@ 2019-03-06 23:05 ` Cohen, Eugene
2019-03-06 23:06 ` Ashish Singhal
1 sibling, 1 reply; 9+ messages in thread
From: Cohen, Eugene @ 2019-03-06 23:05 UTC (permalink / raw)
To: Wu, Hao A, Ashish Singhal, edk2-devel@lists.01.org
Ø I verified the patch on SDHC version 3.00 with 64-bit System Address
Ø Support. Hope more configurations are available for testing on Eugene's
Ø side.
This patch works for us. Tested that the V3 64-bit DMA works and verified that addresses above 4GB DMA correctly.
Thanks for putting this together.
Feel free to add my Tested-By.
Eugene
From: Wu, Hao A <hao.a.wu@intel.com>
Sent: Tuesday, March 5, 2019 8:01 PM
To: Ashish Singhal <ashishsingha@nvidia.com>; edk2-devel@lists.01.org
Cc: Cohen, Eugene <eugene@hp.com>
Subject: RE: [PATCH] MdeModulePkg/SdMmcPciHcDxe: Add V3 64b DMA Support
Hi Ashish,
One thing to confirm, for the updated checks within
SdMmcPciHcDriverBindingStart():
> if ((Private->ControllerVersion[Slot] == SD_MMC_HC_CTRL_VER_300 &&
> Private->Capability[Slot].SysBus64V3 == 0) ||
> (Private->ControllerVersion[Slot] == SD_MMC_HC_CTRL_VER_400 &&
> Private->Capability[Slot].SysBus64V3 == 0) ||
> (Private->ControllerVersion[Slot] >= SD_MMC_HC_CTRL_VER_410 &&
> Private->Capability[Slot].SysBus64V4 == 0)) {
> Support64BitDma = FALSE;
> }
When the SDHC with version greater than 4.10, the check is only performed
against the 'SysBus64V4' bit. My understanding of the purpose is that:
1. For SDHC with version 4.00, the support of V3 mode and V4 mode of
64-bit System Address are reflect by bit 'SysBus64V3'. Thus, I can infer
that the possible support case is both or neither.
2. The spec states that SDHC with version greater than 4.10 divides the V3
mode and V4 mode support into 2 bits (SysBus64V3, SysBus64V4) so that the
V3 mode support can be optional.
So based on 1 & 2, we do not even bother to check the 'SysBus64V3' bit
when HC version >= 4.10. Is that right?
I verified the patch on SDHC version 3.00 with 64-bit System Address
Support. Hope more configurations are available for testing on Eugene's
side.
Besides, some minor comments below:
> -----Original Message-----
> From: Ashish Singhal [mailto:ashishsingha@nvidia.com]
> Sent: Saturday, March 02, 2019 2:30 AM
> To: edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
> Cc: Wu, Hao A; eugene@hp.com<mailto:eugene@hp.com>; Ashish Singhal
> Subject: [PATCH] MdeModulePkg/SdMmcPciHcDxe: Add V3 64b DMA
> Support
Please help to add the below Bugzilla tracker for reference:
https://bugzilla.tianocore.org/show_bug.cgi?id=1583<https://bugzilla.tianocore.org/show_bug.cgi?id=1583>
I have updated the above tracker to match the purpose of the proposed
patch.
>
> Driver was supporting only 32b DMA support for V3 controllers. Add
> support for 64b DMA as well for completeness.
>
> For V4.0 64b support, driver was looking at incorrect capability
> register bit. Fix for that is present as well.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com>>
> ---
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c | 10 +-
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h | 6 +-
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c | 199
> ++++++++++++++-------
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h | 29 ++-
> 4 files changed, 170 insertions(+), 74 deletions(-)
>
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> index b474f8d..9b7b88c 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> @@ -6,7 +6,7 @@
>
> It would expose EFI_SD_MMC_PASS_THRU_PROTOCOL for upper layer use.
>
> - Copyright (c) 2018, NVIDIA CORPORATION. All rights reserved.
> + Copyright (c) 2018-2019, NVIDIA CORPORATION. All rights reserved.
> Copyright (c) 2015 - 2019, Intel Corporation. All rights reserved.<BR>
> This program and the accompanying materials
> are licensed and made available under the terms and conditions of the BSD
> License
> @@ -666,8 +666,12 @@ SdMmcPciHcDriverBindingStart (
> // If any of the slots does not support 64b system bus
> // do not enable 64b DMA in the PCI layer.
> //
> - if (Private->Capability[Slot].SysBus64V3 == 0 &&
> - Private->Capability[Slot].SysBus64V4 == 0) {
> + if ((Private->ControllerVersion[Slot] == SD_MMC_HC_CTRL_VER_300 &&
> + Private->Capability[Slot].SysBus64V3 == 0) ||
> + (Private->ControllerVersion[Slot] == SD_MMC_HC_CTRL_VER_400 &&
> + Private->Capability[Slot].SysBus64V3 == 0) ||
> + (Private->ControllerVersion[Slot] >= SD_MMC_HC_CTRL_VER_410 &&
> + Private->Capability[Slot].SysBus64V4 == 0)) {
> Support64BitDma = FALSE;
> }
>
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> index 1bb701a..68d8a5c 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> @@ -2,7 +2,7 @@
>
> Provides some data structure definitions used by the SD/MMC host
> controller driver.
>
> -Copyright (c) 2018, NVIDIA CORPORATION. All rights reserved.
> +Copyright (c) 2018-2019, NVIDIA CORPORATION. All rights reserved.
> Copyright (c) 2015, Intel Corporation. All rights reserved.<BR>
> This program and the accompanying materials
> are licensed and made available under the terms and conditions of the BSD
> License
> @@ -145,13 +145,15 @@ typedef struct {
> EFI_PHYSICAL_ADDRESS DataPhy;
> VOID *DataMap;
> SD_MMC_HC_TRANSFER_MODE Mode;
> + SD_MMC_HC_ADMA_LEGTH Length;
Maybe:
SD_MMC_HC_ADMA_LENGTH_MODE AdmaLengthMode;
is better to avoid confusion.
>
> EFI_EVENT Event;
> BOOLEAN Started;
> UINT64 Timeout;
>
> SD_MMC_HC_ADMA_32_DESC_LINE *Adma32Desc;
> - SD_MMC_HC_ADMA_64_DESC_LINE *Adma64Desc;
> + SD_MMC_HC_ADMA_64_V3_DESC_LINE *Adma64V3Desc;
> + SD_MMC_HC_ADMA_64_V4_DESC_LINE *Adma64V4Desc;
> EFI_PHYSICAL_ADDRESS AdmaDescPhy;
> VOID *AdmaMap;
> UINT32 AdmaPages;
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> index d73fa10..a6d2395 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> @@ -6,7 +6,7 @@
>
> It would expose EFI_SD_MMC_PASS_THRU_PROTOCOL for upper layer use.
>
> - Copyright (c) 2018, NVIDIA CORPORATION. All rights reserved.
> + Copyright (c) 2018-2019, NVIDIA CORPORATION. All rights reserved.
> Copyright (c) 2015 - 2019, Intel Corporation. All rights reserved.<BR>
> This program and the accompanying materials
> are licensed and made available under the terms and conditions of the BSD
> License
> @@ -1010,18 +1010,32 @@ SdMmcHcInitV4Enhancements (
> if (ControllerVer >= SD_MMC_HC_CTRL_VER_400) {
> HostCtrl2 = SD_MMC_HC_V4_EN;
> //
> - // Check if V4 64bit support is available
> + // Check if controller version V4.0
> //
> - if (Capability.SysBus64V4 != 0) {
> - HostCtrl2 |= SD_MMC_HC_64_ADDR_EN;
> - DEBUG ((DEBUG_INFO, "Enabled V4 64 bit system bus support\n"));
> + if (ControllerVer == SD_MMC_HC_CTRL_VER_400) {
> + //
> + // Check if 64bit support is available
> + //
> + if (Capability.SysBus64V3 != 0) {
> + HostCtrl2 |= SD_MMC_HC_64_ADDR_EN;
> + DEBUG ((DEBUG_INFO, "Enabled V4 64 bit system bus support\n"));
> + }
> }
> //
> // Check if controller version V4.10 or higher
> //
> - if (ControllerVer >= SD_MMC_HC_CTRL_VER_410) {
> - HostCtrl2 |= SD_MMC_HC_26_DATA_LEN_ADMA_EN;
> - DEBUG ((DEBUG_INFO, "Enabled V4 26 bit data length ADMA
> support\n"));
> + else if (ControllerVer >= SD_MMC_HC_CTRL_VER_410) {
> + //
> + // Check if 64bit support is available
> + //
> + if (Capability.SysBus64V4 != 0) {
> + HostCtrl2 |= SD_MMC_HC_64_ADDR_EN;
> + DEBUG ((DEBUG_INFO, "Enabled V4 64 bit system bus support\n"));
> + }
> + if (ControllerVer >= SD_MMC_HC_CTRL_VER_410) {
I think the above 'if' statement can be removed.
> + HostCtrl2 |= SD_MMC_HC_26_DATA_LEN_ADMA_EN;
> + DEBUG ((DEBUG_INFO, "Enabled V4 26 bit data length ADMA
> support\n"));
> + }
> }
> Status = SdMmcHcOrMmio (PciIo, Slot, SD_MMC_HC_HOST_CTRL2, sizeof
> (HostCtrl2), &HostCtrl2);
> if (EFI_ERROR (Status)) {
> @@ -1393,14 +1407,10 @@ BuildAdmaDescTable (
> EFI_PCI_IO_PROTOCOL *PciIo;
> EFI_STATUS Status;
> UINTN Bytes;
> - BOOLEAN AddressingMode64;
> - BOOLEAN DataLength26;
> UINT32 AdmaMaxDataPerLine;
> UINT32 DescSize;
> VOID *AdmaDesc;
>
> - AddressingMode64 = FALSE;
> - DataLength26 = FALSE;
> AdmaMaxDataPerLine = ADMA_MAX_DATA_PER_LINE_16B;
> DescSize = sizeof (SD_MMC_HC_ADMA_32_DESC_LINE);
> AdmaDesc = NULL;
> @@ -1410,27 +1420,16 @@ BuildAdmaDescTable (
> PciIo = Trb->Private->PciIo;
>
> //
> - // Detect whether 64bit addressing is supported.
> - //
> - if (ControllerVer >= SD_MMC_HC_CTRL_VER_400) {
> - Status = SdMmcHcCheckMmioSet(PciIo, Trb->Slot,
> SD_MMC_HC_HOST_CTRL2, sizeof(UINT16),
> - SD_MMC_HC_V4_EN|SD_MMC_HC_64_ADDR_EN,
> SD_MMC_HC_V4_EN|SD_MMC_HC_64_ADDR_EN);
> - if (!EFI_ERROR (Status)) {
> - AddressingMode64 = TRUE;
> - DescSize = sizeof (SD_MMC_HC_ADMA_64_DESC_LINE);
> - }
> - }
> - //
> // Check for valid ranges in 32bit ADMA Descriptor Table
> //
> - if (!AddressingMode64 &&
> + if ((Trb->Mode == SdMmcAdma32bMode) &&
> ((Data >= 0x100000000ul) || ((Data + DataLen) > 0x100000000ul))) {
> return EFI_INVALID_PARAMETER;
> }
> //
> // Check address field alignment
> //
> - if (AddressingMode64) {
> + if (Trb->Mode != SdMmcAdma32bMode) {
> //
> // Address field shall be set on 64-bit boundary (Lower 3-bit is always set
> to 0)
> //
> @@ -1445,13 +1444,19 @@ BuildAdmaDescTable (
> DEBUG ((DEBUG_INFO, "The buffer [0x%x] to construct ADMA desc is not
> aligned to 4 bytes boundary!\n", Data));
> }
> }
> +
> //
> - // Detect whether 26bit data length is supported.
> + // Configure 64b ADMA.
> //
> - Status = SdMmcHcCheckMmioSet(PciIo, Trb->Slot,
> SD_MMC_HC_HOST_CTRL2, sizeof(UINT16),
> - SD_MMC_HC_26_DATA_LEN_ADMA_EN,
> SD_MMC_HC_26_DATA_LEN_ADMA_EN);
> - if (!EFI_ERROR (Status)) {
> - DataLength26 = TRUE;
> + if (Trb->Mode == SdMmcAdma64bV3Mode) {
> + DescSize = sizeof (SD_MMC_HC_ADMA_64_V3_DESC_LINE);
> + }else if (Trb->Mode == SdMmcAdma64bV4Mode) {
> + DescSize = sizeof (SD_MMC_HC_ADMA_64_V4_DESC_LINE);
> + }
> + //
> + // Configure 26b data length.
> + //
> + if (Trb->Length == SdMmcAdmaLen26b) {
> AdmaMaxDataPerLine = ADMA_MAX_DATA_PER_LINE_26B;
> }
>
> @@ -1492,7 +1497,7 @@ BuildAdmaDescTable (
> return EFI_OUT_OF_RESOURCES;
> }
>
> - if ((!AddressingMode64) &&
> + if ((Trb->Mode == SdMmcAdma32bMode) &&
> (UINT64)(UINTN)Trb->AdmaDescPhy > 0x100000000ul) {
> //
> // The ADMA doesn't support 64bit addressing.
> @@ -1511,19 +1516,26 @@ BuildAdmaDescTable (
>
> Remaining = DataLen;
> Address = Data;
> - if (!AddressingMode64) {
> + if (Trb->Mode == SdMmcAdma32bMode) {
> Trb->Adma32Desc = AdmaDesc;
> - Trb->Adma64Desc = NULL;
> + Trb->Adma64V3Desc = NULL;
> + Trb->Adma64V4Desc = NULL;
> + } else if (Trb->Mode == SdMmcAdma64bV3Mode) {
> + Trb->Adma64V3Desc = AdmaDesc;
> + Trb->Adma32Desc = NULL;
> + Trb->Adma64V4Desc = NULL;
> } else {
> - Trb->Adma64Desc = AdmaDesc;
> + Trb->Adma64V4Desc = AdmaDesc;
> Trb->Adma32Desc = NULL;
> + Trb->Adma64V3Desc = NULL;
> }
Suggest to remove those NULL assignments to:
Trb->Adma32Desc
Trb->Adma64Desc
Trb->Adma64V3Desc
Since in SdMmcCreateTrb(), 'Trb' is allocated by:
Trb = AllocateZeroPool (sizeof (SD_MMC_HC_TRB));
> +
> for (Index = 0; Index < Entries; Index++) {
> - if (!AddressingMode64) {
> + if (Trb->Mode == SdMmcAdma32bMode) {
> if (Remaining <= AdmaMaxDataPerLine) {
> Trb->Adma32Desc[Index].Valid = 1;
> Trb->Adma32Desc[Index].Act = 2;
> - if (DataLength26) {
> + if (Trb->Length == SdMmcAdmaLen26b) {
> Trb->Adma32Desc[Index].UpperLength = (UINT16)RShiftU64
> (Remaining, 16);
> }
> Trb->Adma32Desc[Index].LowerLength = (UINT16)(Remaining &
> MAX_UINT16);
> @@ -1532,32 +1544,53 @@ BuildAdmaDescTable (
> } else {
> Trb->Adma32Desc[Index].Valid = 1;
> Trb->Adma32Desc[Index].Act = 2;
> - if (DataLength26) {
> + if (Trb->Length == SdMmcAdmaLen26b) {
> Trb->Adma32Desc[Index].UpperLength = 0;
> }
> Trb->Adma32Desc[Index].LowerLength = 0;
> Trb->Adma32Desc[Index].Address = (UINT32)Address;
> }
> + } else if (Trb->Mode == SdMmcAdma64bV3Mode) {
> + if (Remaining <= AdmaMaxDataPerLine) {
> + Trb->Adma64V3Desc[Index].Valid = 1;
> + Trb->Adma64V3Desc[Index].Act = 2;
> + if (Trb->Length == SdMmcAdmaLen26b) {
> + Trb->Adma64V3Desc[Index].UpperLength = (UINT16)RShiftU64
> (Remaining, 16);
> + }
> + Trb->Adma64V3Desc[Index].LowerLength = (UINT16)(Remaining &
> MAX_UINT16);
> + Trb->Adma64V3Desc[Index].LowerAddress = (UINT32)Address;
> + Trb->Adma64V3Desc[Index].UpperAddress = (UINT32)RShiftU64
> (Address, 32);
> + break;
> + } else {
> + Trb->Adma64V3Desc[Index].Valid = 1;
> + Trb->Adma64V3Desc[Index].Act = 2;
> + if (Trb->Length == SdMmcAdmaLen26b) {
> + Trb->Adma64V3Desc[Index].UpperLength = 0;
> + }
> + Trb->Adma64V3Desc[Index].LowerLength = 0;
> + Trb->Adma64V3Desc[Index].LowerAddress = (UINT32)Address;
> + Trb->Adma64V3Desc[Index].UpperAddress = (UINT32)RShiftU64
> (Address, 32);
> + }
> } else {
> if (Remaining <= AdmaMaxDataPerLine) {
> - Trb->Adma64Desc[Index].Valid = 1;
> - Trb->Adma64Desc[Index].Act = 2;
> - if (DataLength26) {
> - Trb->Adma64Desc[Index].UpperLength = (UINT16)RShiftU64
> (Remaining, 16);
> + Trb->Adma64V4Desc[Index].Valid = 1;
> + Trb->Adma64V4Desc[Index].Act = 2;
> + if (Trb->Length == SdMmcAdmaLen26b) {
> + Trb->Adma64V4Desc[Index].UpperLength = (UINT16)RShiftU64
> (Remaining, 16);
> }
> - Trb->Adma64Desc[Index].LowerLength = (UINT16)(Remaining &
> MAX_UINT16);
> - Trb->Adma64Desc[Index].LowerAddress = (UINT32)Address;
> - Trb->Adma64Desc[Index].UpperAddress = (UINT32)RShiftU64 (Address,
> 32);
> + Trb->Adma64V4Desc[Index].LowerLength = (UINT16)(Remaining &
> MAX_UINT16);
> + Trb->Adma64V4Desc[Index].LowerAddress = (UINT32)Address;
> + Trb->Adma64V4Desc[Index].UpperAddress = (UINT32)RShiftU64
> (Address, 32);
> break;
> } else {
> - Trb->Adma64Desc[Index].Valid = 1;
> - Trb->Adma64Desc[Index].Act = 2;
> - if (DataLength26) {
> - Trb->Adma64Desc[Index].UpperLength = 0;
> + Trb->Adma64V4Desc[Index].Valid = 1;
> + Trb->Adma64V4Desc[Index].Act = 2;
> + if (Trb->Length == SdMmcAdmaLen26b) {
> + Trb->Adma64V4Desc[Index].UpperLength = 0;
> }
> - Trb->Adma64Desc[Index].LowerLength = 0;
> - Trb->Adma64Desc[Index].LowerAddress = (UINT32)Address;
> - Trb->Adma64Desc[Index].UpperAddress = (UINT32)RShiftU64 (Address,
> 32);
> + Trb->Adma64V4Desc[Index].LowerLength = 0;
> + Trb->Adma64V4Desc[Index].LowerAddress = (UINT32)Address;
> + Trb->Adma64V4Desc[Index].UpperAddress = (UINT32)RShiftU64
> (Address, 32);
> }
> }
>
> @@ -1568,7 +1601,13 @@ BuildAdmaDescTable (
> //
> // Set the last descriptor line as end of descriptor table
> //
> - AddressingMode64 ? (Trb->Adma64Desc[Index].End = 1) : (Trb-
> >Adma32Desc[Index].End = 1);
> + if (Trb->Mode == SdMmcAdma32bMode) {
> + Trb->Adma32Desc[Index].End = 1;
> + } else if (Trb->Mode == SdMmcAdma64bV3Mode) {
> + Trb->Adma64V3Desc[Index].End = 1;
> + } else {
> + Trb->Adma64V4Desc[Index].End = 1;
> + }
> return EFI_SUCCESS;
> }
>
> @@ -1665,7 +1704,20 @@ SdMmcCreateTrb (
> if (Trb->DataLen == 0) {
> Trb->Mode = SdMmcNoData;
> } else if (Private->Capability[Slot].Adma2 != 0) {
> - Trb->Mode = SdMmcAdmaMode;
> + Trb->Mode = SdMmcAdma32bMode;
> + Trb->Length = SdMmcAdmaLen16b;
> + if ((Private->ControllerVersion[Slot] == SD_MMC_HC_CTRL_VER_300)
> &&
> + (Private->Capability[Slot].SysBus64V3 == 1)) {
> + Trb->Mode = SdMmcAdma64bV3Mode;
> + } else if (((Private->ControllerVersion[Slot] ==
> SD_MMC_HC_CTRL_VER_400) &&
> + (Private->Capability[Slot].SysBus64V3 == 1)) ||
> + ((Private->ControllerVersion[Slot] >= SD_MMC_HC_CTRL_VER_410)
> &&
> + (Private->Capability[Slot].SysBus64V4 == 1))) {
> + Trb->Mode = SdMmcAdma64bV4Mode;
> + }
> + if (Private->ControllerVersion[Slot] >= SD_MMC_HC_CTRL_VER_410) {
> + Trb->Length = SdMmcAdmaLen26b;
> + }
> Status = BuildAdmaDescTable (Trb, Private->ControllerVersion[Slot]);
> if (EFI_ERROR (Status)) {
> PciIo->Unmap (PciIo, Trb->DataMap);
> @@ -1719,11 +1771,18 @@ SdMmcFreeTrb (
> Trb->Adma32Desc
> );
> }
> - if (Trb->Adma64Desc != NULL) {
> + if (Trb->Adma64V3Desc != NULL) {
> + PciIo->FreeBuffer (
> + PciIo,
> + Trb->AdmaPages,
> + Trb->Adma64V3Desc
> + );
> + }
> + if (Trb->Adma64V4Desc != NULL) {
> PciIo->FreeBuffer (
> PciIo,
> Trb->AdmaPages,
> - Trb->Adma64Desc
> + Trb->Adma64V4Desc
> );
> }
> if (Trb->DataMap != NULL) {
> @@ -1891,27 +1950,35 @@ SdMmcExecTrb (
> if (EFI_ERROR (Status)) {
> return Status;
> }
> +
> + if (Private->ControllerVersion[Trb->Slot] >= SD_MMC_HC_CTRL_VER_400)
> {
> + Status = SdMmcHcCheckMmioSet(PciIo, Trb->Slot,
> SD_MMC_HC_HOST_CTRL2, sizeof(UINT16),
> + SD_MMC_HC_64_ADDR_EN, SD_MMC_HC_64_ADDR_EN);
> + if (!EFI_ERROR (Status)) {
> + AddressingMode64 = TRUE;
> + }
> + }
> +
> //
> // Set Host Control 1 register DMA Select field
> //
> - if (Trb->Mode == SdMmcAdmaMode) {
> + if ((Trb->Mode == SdMmcAdma32bMode) ||
> + (Trb->Mode == SdMmcAdma64bV4Mode)) {
> HostCtrl1 = BIT4;
> Status = SdMmcHcOrMmio (PciIo, Trb->Slot, SD_MMC_HC_HOST_CTRL1,
> sizeof (HostCtrl1), &HostCtrl1);
> if (EFI_ERROR (Status)) {
> return Status;
> }
> + } else if (Trb->Mode == SdMmcAdma64bV3Mode) {
> + HostCtrl1 = BIT4|BIT3;
> + Status = SdMmcHcOrMmio (PciIo, Trb->Slot, SD_MMC_HC_HOST_CTRL1,
> sizeof (HostCtrl1), &HostCtrl1);
> + if (EFI_ERROR (Status)) {
> + return Status;
> + }
> }
>
> SdMmcHcLedOnOff (PciIo, Trb->Slot, TRUE);
>
> - if (Private->ControllerVersion[Trb->Slot] >= SD_MMC_HC_CTRL_VER_400)
> {
> - Status = SdMmcHcCheckMmioSet(PciIo, Trb->Slot,
> SD_MMC_HC_HOST_CTRL2, sizeof(UINT16),
> - SD_MMC_HC_V4_EN|SD_MMC_HC_64_ADDR_EN,
> SD_MMC_HC_V4_EN|SD_MMC_HC_64_ADDR_EN);
> - if (!EFI_ERROR (Status)) {
> - AddressingMode64 = TRUE;
> - }
> - }
> -
> if (Trb->Mode == SdMmcSdmaMode) {
> if ((!AddressingMode64) &&
> ((UINT64)(UINTN)Trb->DataPhy >= 0x100000000ul)) {
> @@ -1929,7 +1996,9 @@ SdMmcExecTrb (
> if (EFI_ERROR (Status)) {
> return Status;
> }
> - } else if (Trb->Mode == SdMmcAdmaMode) {
> + } else if ((Trb->Mode == SdMmcAdma32bMode) ||
> + (Trb->Mode == SdMmcAdma64bV3Mode) ||
> + (Trb->Mode == SdMmcAdma64bV4Mode)) {
> AdmaAddr = (UINT64)(UINTN)Trb->AdmaDescPhy;
> Status = SdMmcHcRwMmio (PciIo, Trb->Slot,
> SD_MMC_HC_ADMA_SYS_ADDR, FALSE, sizeof (AdmaAddr), &AdmaAddr);
> if (EFI_ERROR (Status)) {
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> index d157f2c..3a05456 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> @@ -2,7 +2,7 @@
>
> Provides some data structure definitions used by the SD/MMC host
> controller driver.
>
> -Copyright (c) 2018, NVIDIA CORPORATION. All rights reserved.
> +Copyright (c) 2018-2019, NVIDIA CORPORATION. All rights reserved.
> Copyright (c) 2015, Intel Corporation. All rights reserved.<BR>
> This program and the accompanying materials
> are licensed and made available under the terms and conditions of the BSD
> License
> @@ -80,16 +80,25 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF
> ANY KIND, EITHER EXPRESS OR IMPLIED.
>
> //
> // The transfer modes supported by SD Host Controller
> -// Simplified Spec 3.0 Table 1-2
> //
> typedef enum {
> SdMmcNoData,
> SdMmcPioMode,
> SdMmcSdmaMode,
> - SdMmcAdmaMode
> + SdMmcAdma32bMode,
> + SdMmcAdma64bV3Mode,
> + SdMmcAdma64bV4Mode
> } SD_MMC_HC_TRANSFER_MODE;
>
> //
> +// The ADMA transfer lengths supported by SD Host Controller
> +//
> +typedef enum {
> + SdMmcAdmaLen16b,
> + SdMmcAdmaLen26b
> +} SD_MMC_HC_ADMA_LEGTH;
Typo 'LEGTH' -> 'LENGTH'
Also, how about 'SD_MMC_HC_ADMA_LENGTH_MODE'?
Best Regards,
Hao Wu
> +
> +//
> // The maximum data length of each descriptor line
> //
> #define ADMA_MAX_DATA_PER_LINE_16B SIZE_64KB
> @@ -122,8 +131,20 @@ typedef struct {
> UINT32 LowerLength:16;
> UINT32 LowerAddress;
> UINT32 UpperAddress;
> +} SD_MMC_HC_ADMA_64_V3_DESC_LINE;
> +
> +typedef struct {
> + UINT32 Valid:1;
> + UINT32 End:1;
> + UINT32 Int:1;
> + UINT32 Reserved:1;
> + UINT32 Act:2;
> + UINT32 UpperLength:10;
> + UINT32 LowerLength:16;
> + UINT32 LowerAddress;
> + UINT32 UpperAddress;
> UINT32 Reserved1;
> -} SD_MMC_HC_ADMA_64_DESC_LINE;
> +} SD_MMC_HC_ADMA_64_V4_DESC_LINE;
>
> #define SD_MMC_SDMA_BOUNDARY 512 * 1024
> #define SD_MMC_SDMA_ROUND_UP(x, n) (((x) + n) & ~(n - 1))
> --
> 2.7.4
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] MdeModulePkg/SdMmcPciHcDxe: Add V3 64b DMA Support
2019-03-06 23:05 ` Cohen, Eugene
@ 2019-03-06 23:06 ` Ashish Singhal
2019-03-07 0:54 ` Cohen, Eugene
0 siblings, 1 reply; 9+ messages in thread
From: Ashish Singhal @ 2019-03-06 23:06 UTC (permalink / raw)
To: Cohen, Eugene, Wu, Hao A, edk2-devel@lists.01.org
Hi Eugene,
Thanks for confirming. Can you please validate the v2 patch I sent as well for completeness?
Thanks
Ashish
From: Cohen, Eugene <eugene@hp.com>
Sent: Wednesday, March 6, 2019 4:05 PM
To: Wu, Hao A <hao.a.wu@intel.com>; Ashish Singhal <ashishsingha@nvidia.com>; edk2-devel@lists.01.org
Subject: RE: [PATCH] MdeModulePkg/SdMmcPciHcDxe: Add V3 64b DMA Support
* I verified the patch on SDHC version 3.00 with 64-bit System Address
* Support. Hope more configurations are available for testing on Eugene's
* side.
This patch works for us. Tested that the V3 64-bit DMA works and verified that addresses above 4GB DMA correctly.
Thanks for putting this together.
Feel free to add my Tested-By.
Eugene
From: Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>
Sent: Tuesday, March 5, 2019 8:01 PM
To: Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com>>; edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
Cc: Cohen, Eugene <eugene@hp.com<mailto:eugene@hp.com>>
Subject: RE: [PATCH] MdeModulePkg/SdMmcPciHcDxe: Add V3 64b DMA Support
Hi Ashish,
One thing to confirm, for the updated checks within
SdMmcPciHcDriverBindingStart():
> if ((Private->ControllerVersion[Slot] == SD_MMC_HC_CTRL_VER_300 &&
> Private->Capability[Slot].SysBus64V3 == 0) ||
> (Private->ControllerVersion[Slot] == SD_MMC_HC_CTRL_VER_400 &&
> Private->Capability[Slot].SysBus64V3 == 0) ||
> (Private->ControllerVersion[Slot] >= SD_MMC_HC_CTRL_VER_410 &&
> Private->Capability[Slot].SysBus64V4 == 0)) {
> Support64BitDma = FALSE;
> }
When the SDHC with version greater than 4.10, the check is only performed
against the 'SysBus64V4' bit. My understanding of the purpose is that:
1. For SDHC with version 4.00, the support of V3 mode and V4 mode of
64-bit System Address are reflect by bit 'SysBus64V3'. Thus, I can infer
that the possible support case is both or neither.
2. The spec states that SDHC with version greater than 4.10 divides the V3
mode and V4 mode support into 2 bits (SysBus64V3, SysBus64V4) so that the
V3 mode support can be optional.
So based on 1 & 2, we do not even bother to check the 'SysBus64V3' bit
when HC version >= 4.10. Is that right?
I verified the patch on SDHC version 3.00 with 64-bit System Address
Support. Hope more configurations are available for testing on Eugene's
side.
Besides, some minor comments below:
> -----Original Message-----
> From: Ashish Singhal [mailto:ashishsingha@nvidia.com]
> Sent: Saturday, March 02, 2019 2:30 AM
> To: edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
> Cc: Wu, Hao A; eugene@hp.com<mailto:eugene@hp.com>; Ashish Singhal
> Subject: [PATCH] MdeModulePkg/SdMmcPciHcDxe: Add V3 64b DMA
> Support
Please help to add the below Bugzilla tracker for reference:
https://bugzilla.tianocore.org/show_bug.cgi?id=1583
I have updated the above tracker to match the purpose of the proposed
patch.
>
> Driver was supporting only 32b DMA support for V3 controllers. Add
> support for 64b DMA as well for completeness.
>
> For V4.0 64b support, driver was looking at incorrect capability
> register bit. Fix for that is present as well.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com>>
> ---
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c | 10 +-
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h | 6 +-
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c | 199
> ++++++++++++++-------
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h | 29 ++-
> 4 files changed, 170 insertions(+), 74 deletions(-)
>
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> index b474f8d..9b7b88c 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> @@ -6,7 +6,7 @@
>
> It would expose EFI_SD_MMC_PASS_THRU_PROTOCOL for upper layer use.
>
> - Copyright (c) 2018, NVIDIA CORPORATION. All rights reserved.
> + Copyright (c) 2018-2019, NVIDIA CORPORATION. All rights reserved.
> Copyright (c) 2015 - 2019, Intel Corporation. All rights reserved.<BR>
> This program and the accompanying materials
> are licensed and made available under the terms and conditions of the BSD
> License
> @@ -666,8 +666,12 @@ SdMmcPciHcDriverBindingStart (
> // If any of the slots does not support 64b system bus
> // do not enable 64b DMA in the PCI layer.
> //
> - if (Private->Capability[Slot].SysBus64V3 == 0 &&
> - Private->Capability[Slot].SysBus64V4 == 0) {
> + if ((Private->ControllerVersion[Slot] == SD_MMC_HC_CTRL_VER_300 &&
> + Private->Capability[Slot].SysBus64V3 == 0) ||
> + (Private->ControllerVersion[Slot] == SD_MMC_HC_CTRL_VER_400 &&
> + Private->Capability[Slot].SysBus64V3 == 0) ||
> + (Private->ControllerVersion[Slot] >= SD_MMC_HC_CTRL_VER_410 &&
> + Private->Capability[Slot].SysBus64V4 == 0)) {
> Support64BitDma = FALSE;
> }
>
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> index 1bb701a..68d8a5c 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> @@ -2,7 +2,7 @@
>
> Provides some data structure definitions used by the SD/MMC host
> controller driver.
>
> -Copyright (c) 2018, NVIDIA CORPORATION. All rights reserved.
> +Copyright (c) 2018-2019, NVIDIA CORPORATION. All rights reserved.
> Copyright (c) 2015, Intel Corporation. All rights reserved.<BR>
> This program and the accompanying materials
> are licensed and made available under the terms and conditions of the BSD
> License
> @@ -145,13 +145,15 @@ typedef struct {
> EFI_PHYSICAL_ADDRESS DataPhy;
> VOID *DataMap;
> SD_MMC_HC_TRANSFER_MODE Mode;
> + SD_MMC_HC_ADMA_LEGTH Length;
Maybe:
SD_MMC_HC_ADMA_LENGTH_MODE AdmaLengthMode;
is better to avoid confusion.
>
> EFI_EVENT Event;
> BOOLEAN Started;
> UINT64 Timeout;
>
> SD_MMC_HC_ADMA_32_DESC_LINE *Adma32Desc;
> - SD_MMC_HC_ADMA_64_DESC_LINE *Adma64Desc;
> + SD_MMC_HC_ADMA_64_V3_DESC_LINE *Adma64V3Desc;
> + SD_MMC_HC_ADMA_64_V4_DESC_LINE *Adma64V4Desc;
> EFI_PHYSICAL_ADDRESS AdmaDescPhy;
> VOID *AdmaMap;
> UINT32 AdmaPages;
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> index d73fa10..a6d2395 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> @@ -6,7 +6,7 @@
>
> It would expose EFI_SD_MMC_PASS_THRU_PROTOCOL for upper layer use.
>
> - Copyright (c) 2018, NVIDIA CORPORATION. All rights reserved.
> + Copyright (c) 2018-2019, NVIDIA CORPORATION. All rights reserved.
> Copyright (c) 2015 - 2019, Intel Corporation. All rights reserved.<BR>
> This program and the accompanying materials
> are licensed and made available under the terms and conditions of the BSD
> License
> @@ -1010,18 +1010,32 @@ SdMmcHcInitV4Enhancements (
> if (ControllerVer >= SD_MMC_HC_CTRL_VER_400) {
> HostCtrl2 = SD_MMC_HC_V4_EN;
> //
> - // Check if V4 64bit support is available
> + // Check if controller version V4.0
> //
> - if (Capability.SysBus64V4 != 0) {
> - HostCtrl2 |= SD_MMC_HC_64_ADDR_EN;
> - DEBUG ((DEBUG_INFO, "Enabled V4 64 bit system bus support\n"));
> + if (ControllerVer == SD_MMC_HC_CTRL_VER_400) {
> + //
> + // Check if 64bit support is available
> + //
> + if (Capability.SysBus64V3 != 0) {
> + HostCtrl2 |= SD_MMC_HC_64_ADDR_EN;
> + DEBUG ((DEBUG_INFO, "Enabled V4 64 bit system bus support\n"));
> + }
> }
> //
> // Check if controller version V4.10 or higher
> //
> - if (ControllerVer >= SD_MMC_HC_CTRL_VER_410) {
> - HostCtrl2 |= SD_MMC_HC_26_DATA_LEN_ADMA_EN;
> - DEBUG ((DEBUG_INFO, "Enabled V4 26 bit data length ADMA
> support\n"));
> + else if (ControllerVer >= SD_MMC_HC_CTRL_VER_410) {
> + //
> + // Check if 64bit support is available
> + //
> + if (Capability.SysBus64V4 != 0) {
> + HostCtrl2 |= SD_MMC_HC_64_ADDR_EN;
> + DEBUG ((DEBUG_INFO, "Enabled V4 64 bit system bus support\n"));
> + }
> + if (ControllerVer >= SD_MMC_HC_CTRL_VER_410) {
I think the above 'if' statement can be removed.
> + HostCtrl2 |= SD_MMC_HC_26_DATA_LEN_ADMA_EN;
> + DEBUG ((DEBUG_INFO, "Enabled V4 26 bit data length ADMA
> support\n"));
> + }
> }
> Status = SdMmcHcOrMmio (PciIo, Slot, SD_MMC_HC_HOST_CTRL2, sizeof
> (HostCtrl2), &HostCtrl2);
> if (EFI_ERROR (Status)) {
> @@ -1393,14 +1407,10 @@ BuildAdmaDescTable (
> EFI_PCI_IO_PROTOCOL *PciIo;
> EFI_STATUS Status;
> UINTN Bytes;
> - BOOLEAN AddressingMode64;
> - BOOLEAN DataLength26;
> UINT32 AdmaMaxDataPerLine;
> UINT32 DescSize;
> VOID *AdmaDesc;
>
> - AddressingMode64 = FALSE;
> - DataLength26 = FALSE;
> AdmaMaxDataPerLine = ADMA_MAX_DATA_PER_LINE_16B;
> DescSize = sizeof (SD_MMC_HC_ADMA_32_DESC_LINE);
> AdmaDesc = NULL;
> @@ -1410,27 +1420,16 @@ BuildAdmaDescTable (
> PciIo = Trb->Private->PciIo;
>
> //
> - // Detect whether 64bit addressing is supported.
> - //
> - if (ControllerVer >= SD_MMC_HC_CTRL_VER_400) {
> - Status = SdMmcHcCheckMmioSet(PciIo, Trb->Slot,
> SD_MMC_HC_HOST_CTRL2, sizeof(UINT16),
> - SD_MMC_HC_V4_EN|SD_MMC_HC_64_ADDR_EN,
> SD_MMC_HC_V4_EN|SD_MMC_HC_64_ADDR_EN);
> - if (!EFI_ERROR (Status)) {
> - AddressingMode64 = TRUE;
> - DescSize = sizeof (SD_MMC_HC_ADMA_64_DESC_LINE);
> - }
> - }
> - //
> // Check for valid ranges in 32bit ADMA Descriptor Table
> //
> - if (!AddressingMode64 &&
> + if ((Trb->Mode == SdMmcAdma32bMode) &&
> ((Data >= 0x100000000ul) || ((Data + DataLen) > 0x100000000ul))) {
> return EFI_INVALID_PARAMETER;
> }
> //
> // Check address field alignment
> //
> - if (AddressingMode64) {
> + if (Trb->Mode != SdMmcAdma32bMode) {
> //
> // Address field shall be set on 64-bit boundary (Lower 3-bit is always set
> to 0)
> //
> @@ -1445,13 +1444,19 @@ BuildAdmaDescTable (
> DEBUG ((DEBUG_INFO, "The buffer [0x%x] to construct ADMA desc is not
> aligned to 4 bytes boundary!\n", Data));
> }
> }
> +
> //
> - // Detect whether 26bit data length is supported.
> + // Configure 64b ADMA.
> //
> - Status = SdMmcHcCheckMmioSet(PciIo, Trb->Slot,
> SD_MMC_HC_HOST_CTRL2, sizeof(UINT16),
> - SD_MMC_HC_26_DATA_LEN_ADMA_EN,
> SD_MMC_HC_26_DATA_LEN_ADMA_EN);
> - if (!EFI_ERROR (Status)) {
> - DataLength26 = TRUE;
> + if (Trb->Mode == SdMmcAdma64bV3Mode) {
> + DescSize = sizeof (SD_MMC_HC_ADMA_64_V3_DESC_LINE);
> + }else if (Trb->Mode == SdMmcAdma64bV4Mode) {
> + DescSize = sizeof (SD_MMC_HC_ADMA_64_V4_DESC_LINE);
> + }
> + //
> + // Configure 26b data length.
> + //
> + if (Trb->Length == SdMmcAdmaLen26b) {
> AdmaMaxDataPerLine = ADMA_MAX_DATA_PER_LINE_26B;
> }
>
> @@ -1492,7 +1497,7 @@ BuildAdmaDescTable (
> return EFI_OUT_OF_RESOURCES;
> }
>
> - if ((!AddressingMode64) &&
> + if ((Trb->Mode == SdMmcAdma32bMode) &&
> (UINT64)(UINTN)Trb->AdmaDescPhy > 0x100000000ul) {
> //
> // The ADMA doesn't support 64bit addressing.
> @@ -1511,19 +1516,26 @@ BuildAdmaDescTable (
>
> Remaining = DataLen;
> Address = Data;
> - if (!AddressingMode64) {
> + if (Trb->Mode == SdMmcAdma32bMode) {
> Trb->Adma32Desc = AdmaDesc;
> - Trb->Adma64Desc = NULL;
> + Trb->Adma64V3Desc = NULL;
> + Trb->Adma64V4Desc = NULL;
> + } else if (Trb->Mode == SdMmcAdma64bV3Mode) {
> + Trb->Adma64V3Desc = AdmaDesc;
> + Trb->Adma32Desc = NULL;
> + Trb->Adma64V4Desc = NULL;
> } else {
> - Trb->Adma64Desc = AdmaDesc;
> + Trb->Adma64V4Desc = AdmaDesc;
> Trb->Adma32Desc = NULL;
> + Trb->Adma64V3Desc = NULL;
> }
Suggest to remove those NULL assignments to:
Trb->Adma32Desc
Trb->Adma64Desc
Trb->Adma64V3Desc
Since in SdMmcCreateTrb(), 'Trb' is allocated by:
Trb = AllocateZeroPool (sizeof (SD_MMC_HC_TRB));
> +
> for (Index = 0; Index < Entries; Index++) {
> - if (!AddressingMode64) {
> + if (Trb->Mode == SdMmcAdma32bMode) {
> if (Remaining <= AdmaMaxDataPerLine) {
> Trb->Adma32Desc[Index].Valid = 1;
> Trb->Adma32Desc[Index].Act = 2;
> - if (DataLength26) {
> + if (Trb->Length == SdMmcAdmaLen26b) {
> Trb->Adma32Desc[Index].UpperLength = (UINT16)RShiftU64
> (Remaining, 16);
> }
> Trb->Adma32Desc[Index].LowerLength = (UINT16)(Remaining &
> MAX_UINT16);
> @@ -1532,32 +1544,53 @@ BuildAdmaDescTable (
> } else {
> Trb->Adma32Desc[Index].Valid = 1;
> Trb->Adma32Desc[Index].Act = 2;
> - if (DataLength26) {
> + if (Trb->Length == SdMmcAdmaLen26b) {
> Trb->Adma32Desc[Index].UpperLength = 0;
> }
> Trb->Adma32Desc[Index].LowerLength = 0;
> Trb->Adma32Desc[Index].Address = (UINT32)Address;
> }
> + } else if (Trb->Mode == SdMmcAdma64bV3Mode) {
> + if (Remaining <= AdmaMaxDataPerLine) {
> + Trb->Adma64V3Desc[Index].Valid = 1;
> + Trb->Adma64V3Desc[Index].Act = 2;
> + if (Trb->Length == SdMmcAdmaLen26b) {
> + Trb->Adma64V3Desc[Index].UpperLength = (UINT16)RShiftU64
> (Remaining, 16);
> + }
> + Trb->Adma64V3Desc[Index].LowerLength = (UINT16)(Remaining &
> MAX_UINT16);
> + Trb->Adma64V3Desc[Index].LowerAddress = (UINT32)Address;
> + Trb->Adma64V3Desc[Index].UpperAddress = (UINT32)RShiftU64
> (Address, 32);
> + break;
> + } else {
> + Trb->Adma64V3Desc[Index].Valid = 1;
> + Trb->Adma64V3Desc[Index].Act = 2;
> + if (Trb->Length == SdMmcAdmaLen26b) {
> + Trb->Adma64V3Desc[Index].UpperLength = 0;
> + }
> + Trb->Adma64V3Desc[Index].LowerLength = 0;
> + Trb->Adma64V3Desc[Index].LowerAddress = (UINT32)Address;
> + Trb->Adma64V3Desc[Index].UpperAddress = (UINT32)RShiftU64
> (Address, 32);
> + }
> } else {
> if (Remaining <= AdmaMaxDataPerLine) {
> - Trb->Adma64Desc[Index].Valid = 1;
> - Trb->Adma64Desc[Index].Act = 2;
> - if (DataLength26) {
> - Trb->Adma64Desc[Index].UpperLength = (UINT16)RShiftU64
> (Remaining, 16);
> + Trb->Adma64V4Desc[Index].Valid = 1;
> + Trb->Adma64V4Desc[Index].Act = 2;
> + if (Trb->Length == SdMmcAdmaLen26b) {
> + Trb->Adma64V4Desc[Index].UpperLength = (UINT16)RShiftU64
> (Remaining, 16);
> }
> - Trb->Adma64Desc[Index].LowerLength = (UINT16)(Remaining &
> MAX_UINT16);
> - Trb->Adma64Desc[Index].LowerAddress = (UINT32)Address;
> - Trb->Adma64Desc[Index].UpperAddress = (UINT32)RShiftU64 (Address,
> 32);
> + Trb->Adma64V4Desc[Index].LowerLength = (UINT16)(Remaining &
> MAX_UINT16);
> + Trb->Adma64V4Desc[Index].LowerAddress = (UINT32)Address;
> + Trb->Adma64V4Desc[Index].UpperAddress = (UINT32)RShiftU64
> (Address, 32);
> break;
> } else {
> - Trb->Adma64Desc[Index].Valid = 1;
> - Trb->Adma64Desc[Index].Act = 2;
> - if (DataLength26) {
> - Trb->Adma64Desc[Index].UpperLength = 0;
> + Trb->Adma64V4Desc[Index].Valid = 1;
> + Trb->Adma64V4Desc[Index].Act = 2;
> + if (Trb->Length == SdMmcAdmaLen26b) {
> + Trb->Adma64V4Desc[Index].UpperLength = 0;
> }
> - Trb->Adma64Desc[Index].LowerLength = 0;
> - Trb->Adma64Desc[Index].LowerAddress = (UINT32)Address;
> - Trb->Adma64Desc[Index].UpperAddress = (UINT32)RShiftU64 (Address,
> 32);
> + Trb->Adma64V4Desc[Index].LowerLength = 0;
> + Trb->Adma64V4Desc[Index].LowerAddress = (UINT32)Address;
> + Trb->Adma64V4Desc[Index].UpperAddress = (UINT32)RShiftU64
> (Address, 32);
> }
> }
>
> @@ -1568,7 +1601,13 @@ BuildAdmaDescTable (
> //
> // Set the last descriptor line as end of descriptor table
> //
> - AddressingMode64 ? (Trb->Adma64Desc[Index].End = 1) : (Trb-
> >Adma32Desc[Index].End = 1);
> + if (Trb->Mode == SdMmcAdma32bMode) {
> + Trb->Adma32Desc[Index].End = 1;
> + } else if (Trb->Mode == SdMmcAdma64bV3Mode) {
> + Trb->Adma64V3Desc[Index].End = 1;
> + } else {
> + Trb->Adma64V4Desc[Index].End = 1;
> + }
> return EFI_SUCCESS;
> }
>
> @@ -1665,7 +1704,20 @@ SdMmcCreateTrb (
> if (Trb->DataLen == 0) {
> Trb->Mode = SdMmcNoData;
> } else if (Private->Capability[Slot].Adma2 != 0) {
> - Trb->Mode = SdMmcAdmaMode;
> + Trb->Mode = SdMmcAdma32bMode;
> + Trb->Length = SdMmcAdmaLen16b;
> + if ((Private->ControllerVersion[Slot] == SD_MMC_HC_CTRL_VER_300)
> &&
> + (Private->Capability[Slot].SysBus64V3 == 1)) {
> + Trb->Mode = SdMmcAdma64bV3Mode;
> + } else if (((Private->ControllerVersion[Slot] ==
> SD_MMC_HC_CTRL_VER_400) &&
> + (Private->Capability[Slot].SysBus64V3 == 1)) ||
> + ((Private->ControllerVersion[Slot] >= SD_MMC_HC_CTRL_VER_410)
> &&
> + (Private->Capability[Slot].SysBus64V4 == 1))) {
> + Trb->Mode = SdMmcAdma64bV4Mode;
> + }
> + if (Private->ControllerVersion[Slot] >= SD_MMC_HC_CTRL_VER_410) {
> + Trb->Length = SdMmcAdmaLen26b;
> + }
> Status = BuildAdmaDescTable (Trb, Private->ControllerVersion[Slot]);
> if (EFI_ERROR (Status)) {
> PciIo->Unmap (PciIo, Trb->DataMap);
> @@ -1719,11 +1771,18 @@ SdMmcFreeTrb (
> Trb->Adma32Desc
> );
> }
> - if (Trb->Adma64Desc != NULL) {
> + if (Trb->Adma64V3Desc != NULL) {
> + PciIo->FreeBuffer (
> + PciIo,
> + Trb->AdmaPages,
> + Trb->Adma64V3Desc
> + );
> + }
> + if (Trb->Adma64V4Desc != NULL) {
> PciIo->FreeBuffer (
> PciIo,
> Trb->AdmaPages,
> - Trb->Adma64Desc
> + Trb->Adma64V4Desc
> );
> }
> if (Trb->DataMap != NULL) {
> @@ -1891,27 +1950,35 @@ SdMmcExecTrb (
> if (EFI_ERROR (Status)) {
> return Status;
> }
> +
> + if (Private->ControllerVersion[Trb->Slot] >= SD_MMC_HC_CTRL_VER_400)
> {
> + Status = SdMmcHcCheckMmioSet(PciIo, Trb->Slot,
> SD_MMC_HC_HOST_CTRL2, sizeof(UINT16),
> + SD_MMC_HC_64_ADDR_EN, SD_MMC_HC_64_ADDR_EN);
> + if (!EFI_ERROR (Status)) {
> + AddressingMode64 = TRUE;
> + }
> + }
> +
> //
> // Set Host Control 1 register DMA Select field
> //
> - if (Trb->Mode == SdMmcAdmaMode) {
> + if ((Trb->Mode == SdMmcAdma32bMode) ||
> + (Trb->Mode == SdMmcAdma64bV4Mode)) {
> HostCtrl1 = BIT4;
> Status = SdMmcHcOrMmio (PciIo, Trb->Slot, SD_MMC_HC_HOST_CTRL1,
> sizeof (HostCtrl1), &HostCtrl1);
> if (EFI_ERROR (Status)) {
> return Status;
> }
> + } else if (Trb->Mode == SdMmcAdma64bV3Mode) {
> + HostCtrl1 = BIT4|BIT3;
> + Status = SdMmcHcOrMmio (PciIo, Trb->Slot, SD_MMC_HC_HOST_CTRL1,
> sizeof (HostCtrl1), &HostCtrl1);
> + if (EFI_ERROR (Status)) {
> + return Status;
> + }
> }
>
> SdMmcHcLedOnOff (PciIo, Trb->Slot, TRUE);
>
> - if (Private->ControllerVersion[Trb->Slot] >= SD_MMC_HC_CTRL_VER_400)
> {
> - Status = SdMmcHcCheckMmioSet(PciIo, Trb->Slot,
> SD_MMC_HC_HOST_CTRL2, sizeof(UINT16),
> - SD_MMC_HC_V4_EN|SD_MMC_HC_64_ADDR_EN,
> SD_MMC_HC_V4_EN|SD_MMC_HC_64_ADDR_EN);
> - if (!EFI_ERROR (Status)) {
> - AddressingMode64 = TRUE;
> - }
> - }
> -
> if (Trb->Mode == SdMmcSdmaMode) {
> if ((!AddressingMode64) &&
> ((UINT64)(UINTN)Trb->DataPhy >= 0x100000000ul)) {
> @@ -1929,7 +1996,9 @@ SdMmcExecTrb (
> if (EFI_ERROR (Status)) {
> return Status;
> }
> - } else if (Trb->Mode == SdMmcAdmaMode) {
> + } else if ((Trb->Mode == SdMmcAdma32bMode) ||
> + (Trb->Mode == SdMmcAdma64bV3Mode) ||
> + (Trb->Mode == SdMmcAdma64bV4Mode)) {
> AdmaAddr = (UINT64)(UINTN)Trb->AdmaDescPhy;
> Status = SdMmcHcRwMmio (PciIo, Trb->Slot,
> SD_MMC_HC_ADMA_SYS_ADDR, FALSE, sizeof (AdmaAddr), &AdmaAddr);
> if (EFI_ERROR (Status)) {
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> index d157f2c..3a05456 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> @@ -2,7 +2,7 @@
>
> Provides some data structure definitions used by the SD/MMC host
> controller driver.
>
> -Copyright (c) 2018, NVIDIA CORPORATION. All rights reserved.
> +Copyright (c) 2018-2019, NVIDIA CORPORATION. All rights reserved.
> Copyright (c) 2015, Intel Corporation. All rights reserved.<BR>
> This program and the accompanying materials
> are licensed and made available under the terms and conditions of the BSD
> License
> @@ -80,16 +80,25 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF
> ANY KIND, EITHER EXPRESS OR IMPLIED.
>
> //
> // The transfer modes supported by SD Host Controller
> -// Simplified Spec 3.0 Table 1-2
> //
> typedef enum {
> SdMmcNoData,
> SdMmcPioMode,
> SdMmcSdmaMode,
> - SdMmcAdmaMode
> + SdMmcAdma32bMode,
> + SdMmcAdma64bV3Mode,
> + SdMmcAdma64bV4Mode
> } SD_MMC_HC_TRANSFER_MODE;
>
> //
> +// The ADMA transfer lengths supported by SD Host Controller
> +//
> +typedef enum {
> + SdMmcAdmaLen16b,
> + SdMmcAdmaLen26b
> +} SD_MMC_HC_ADMA_LEGTH;
Typo 'LEGTH' -> 'LENGTH'
Also, how about 'SD_MMC_HC_ADMA_LENGTH_MODE'?
Best Regards,
Hao Wu
> +
> +//
> // The maximum data length of each descriptor line
> //
> #define ADMA_MAX_DATA_PER_LINE_16B SIZE_64KB
> @@ -122,8 +131,20 @@ typedef struct {
> UINT32 LowerLength:16;
> UINT32 LowerAddress;
> UINT32 UpperAddress;
> +} SD_MMC_HC_ADMA_64_V3_DESC_LINE;
> +
> +typedef struct {
> + UINT32 Valid:1;
> + UINT32 End:1;
> + UINT32 Int:1;
> + UINT32 Reserved:1;
> + UINT32 Act:2;
> + UINT32 UpperLength:10;
> + UINT32 LowerLength:16;
> + UINT32 LowerAddress;
> + UINT32 UpperAddress;
> UINT32 Reserved1;
> -} SD_MMC_HC_ADMA_64_DESC_LINE;
> +} SD_MMC_HC_ADMA_64_V4_DESC_LINE;
>
> #define SD_MMC_SDMA_BOUNDARY 512 * 1024
> #define SD_MMC_SDMA_ROUND_UP(x, n) (((x) + n) & ~(n - 1))
> --
> 2.7.4
-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information. Any unauthorized review, use, disclosure or distribution
is prohibited. If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] MdeModulePkg/SdMmcPciHcDxe: Add V3 64b DMA Support
2019-03-06 23:06 ` Ashish Singhal
@ 2019-03-07 0:54 ` Cohen, Eugene
2019-03-07 1:34 ` Wu, Hao A
0 siblings, 1 reply; 9+ messages in thread
From: Cohen, Eugene @ 2019-03-07 0:54 UTC (permalink / raw)
To: Ashish Singhal, Wu, Hao A, edk2-devel@lists.01.org
Tested-by: Eugene Cohen <eugene@hp.com>
Thanks again.
From: Ashish Singhal <ashishsingha@nvidia.com>
Sent: Wednesday, March 6, 2019 4:07 PM
To: Cohen, Eugene <eugene@hp.com>; Wu, Hao A <hao.a.wu@intel.com>; edk2-devel@lists.01.org
Subject: RE: [PATCH] MdeModulePkg/SdMmcPciHcDxe: Add V3 64b DMA Support
Hi Eugene,
Thanks for confirming. Can you please validate the v2 patch I sent as well for completeness?
Thanks
Ashish
From: Cohen, Eugene <eugene@hp.com<mailto:eugene@hp.com>>
Sent: Wednesday, March 6, 2019 4:05 PM
To: Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>; Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com>>; edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
Subject: RE: [PATCH] MdeModulePkg/SdMmcPciHcDxe: Add V3 64b DMA Support
Ø I verified the patch on SDHC version 3.00 with 64-bit System Address
Ø Support. Hope more configurations are available for testing on Eugene's
Ø side.
This patch works for us. Tested that the V3 64-bit DMA works and verified that addresses above 4GB DMA correctly.
Thanks for putting this together.
Feel free to add my Tested-By.
Eugene
From: Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>
Sent: Tuesday, March 5, 2019 8:01 PM
To: Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com>>; edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
Cc: Cohen, Eugene <eugene@hp.com<mailto:eugene@hp.com>>
Subject: RE: [PATCH] MdeModulePkg/SdMmcPciHcDxe: Add V3 64b DMA Support
Hi Ashish,
One thing to confirm, for the updated checks within
SdMmcPciHcDriverBindingStart():
> if ((Private->ControllerVersion[Slot] == SD_MMC_HC_CTRL_VER_300 &&
> Private->Capability[Slot].SysBus64V3 == 0) ||
> (Private->ControllerVersion[Slot] == SD_MMC_HC_CTRL_VER_400 &&
> Private->Capability[Slot].SysBus64V3 == 0) ||
> (Private->ControllerVersion[Slot] >= SD_MMC_HC_CTRL_VER_410 &&
> Private->Capability[Slot].SysBus64V4 == 0)) {
> Support64BitDma = FALSE;
> }
When the SDHC with version greater than 4.10, the check is only performed
against the 'SysBus64V4' bit. My understanding of the purpose is that:
1. For SDHC with version 4.00, the support of V3 mode and V4 mode of
64-bit System Address are reflect by bit 'SysBus64V3'. Thus, I can infer
that the possible support case is both or neither.
2. The spec states that SDHC with version greater than 4.10 divides the V3
mode and V4 mode support into 2 bits (SysBus64V3, SysBus64V4) so that the
V3 mode support can be optional.
So based on 1 & 2, we do not even bother to check the 'SysBus64V3' bit
when HC version >= 4.10. Is that right?
I verified the patch on SDHC version 3.00 with 64-bit System Address
Support. Hope more configurations are available for testing on Eugene's
side.
Besides, some minor comments below:
> -----Original Message-----
> From: Ashish Singhal [mailto:ashishsingha@nvidia.com]
> Sent: Saturday, March 02, 2019 2:30 AM
> To: edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
> Cc: Wu, Hao A; eugene@hp.com<mailto:eugene@hp.com>; Ashish Singhal
> Subject: [PATCH] MdeModulePkg/SdMmcPciHcDxe: Add V3 64b DMA
> Support
Please help to add the below Bugzilla tracker for reference:
https://bugzilla.tianocore.org/show_bug.cgi?id=1583<https://bugzilla.tianocore.org/show_bug.cgi?id=1583>
I have updated the above tracker to match the purpose of the proposed
patch.
>
> Driver was supporting only 32b DMA support for V3 controllers. Add
> support for 64b DMA as well for completeness.
>
> For V4.0 64b support, driver was looking at incorrect capability
> register bit. Fix for that is present as well.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com>>
> ---
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c | 10 +-
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h | 6 +-
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c | 199
> ++++++++++++++-------
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h | 29 ++-
> 4 files changed, 170 insertions(+), 74 deletions(-)
>
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> index b474f8d..9b7b88c 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> @@ -6,7 +6,7 @@
>
> It would expose EFI_SD_MMC_PASS_THRU_PROTOCOL for upper layer use.
>
> - Copyright (c) 2018, NVIDIA CORPORATION. All rights reserved.
> + Copyright (c) 2018-2019, NVIDIA CORPORATION. All rights reserved.
> Copyright (c) 2015 - 2019, Intel Corporation. All rights reserved.<BR>
> This program and the accompanying materials
> are licensed and made available under the terms and conditions of the BSD
> License
> @@ -666,8 +666,12 @@ SdMmcPciHcDriverBindingStart (
> // If any of the slots does not support 64b system bus
> // do not enable 64b DMA in the PCI layer.
> //
> - if (Private->Capability[Slot].SysBus64V3 == 0 &&
> - Private->Capability[Slot].SysBus64V4 == 0) {
> + if ((Private->ControllerVersion[Slot] == SD_MMC_HC_CTRL_VER_300 &&
> + Private->Capability[Slot].SysBus64V3 == 0) ||
> + (Private->ControllerVersion[Slot] == SD_MMC_HC_CTRL_VER_400 &&
> + Private->Capability[Slot].SysBus64V3 == 0) ||
> + (Private->ControllerVersion[Slot] >= SD_MMC_HC_CTRL_VER_410 &&
> + Private->Capability[Slot].SysBus64V4 == 0)) {
> Support64BitDma = FALSE;
> }
>
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> index 1bb701a..68d8a5c 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> @@ -2,7 +2,7 @@
>
> Provides some data structure definitions used by the SD/MMC host
> controller driver.
>
> -Copyright (c) 2018, NVIDIA CORPORATION. All rights reserved.
> +Copyright (c) 2018-2019, NVIDIA CORPORATION. All rights reserved.
> Copyright (c) 2015, Intel Corporation. All rights reserved.<BR>
> This program and the accompanying materials
> are licensed and made available under the terms and conditions of the BSD
> License
> @@ -145,13 +145,15 @@ typedef struct {
> EFI_PHYSICAL_ADDRESS DataPhy;
> VOID *DataMap;
> SD_MMC_HC_TRANSFER_MODE Mode;
> + SD_MMC_HC_ADMA_LEGTH Length;
Maybe:
SD_MMC_HC_ADMA_LENGTH_MODE AdmaLengthMode;
is better to avoid confusion.
>
> EFI_EVENT Event;
> BOOLEAN Started;
> UINT64 Timeout;
>
> SD_MMC_HC_ADMA_32_DESC_LINE *Adma32Desc;
> - SD_MMC_HC_ADMA_64_DESC_LINE *Adma64Desc;
> + SD_MMC_HC_ADMA_64_V3_DESC_LINE *Adma64V3Desc;
> + SD_MMC_HC_ADMA_64_V4_DESC_LINE *Adma64V4Desc;
> EFI_PHYSICAL_ADDRESS AdmaDescPhy;
> VOID *AdmaMap;
> UINT32 AdmaPages;
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> index d73fa10..a6d2395 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> @@ -6,7 +6,7 @@
>
> It would expose EFI_SD_MMC_PASS_THRU_PROTOCOL for upper layer use.
>
> - Copyright (c) 2018, NVIDIA CORPORATION. All rights reserved.
> + Copyright (c) 2018-2019, NVIDIA CORPORATION. All rights reserved.
> Copyright (c) 2015 - 2019, Intel Corporation. All rights reserved.<BR>
> This program and the accompanying materials
> are licensed and made available under the terms and conditions of the BSD
> License
> @@ -1010,18 +1010,32 @@ SdMmcHcInitV4Enhancements (
> if (ControllerVer >= SD_MMC_HC_CTRL_VER_400) {
> HostCtrl2 = SD_MMC_HC_V4_EN;
> //
> - // Check if V4 64bit support is available
> + // Check if controller version V4.0
> //
> - if (Capability.SysBus64V4 != 0) {
> - HostCtrl2 |= SD_MMC_HC_64_ADDR_EN;
> - DEBUG ((DEBUG_INFO, "Enabled V4 64 bit system bus support\n"));
> + if (ControllerVer == SD_MMC_HC_CTRL_VER_400) {
> + //
> + // Check if 64bit support is available
> + //
> + if (Capability.SysBus64V3 != 0) {
> + HostCtrl2 |= SD_MMC_HC_64_ADDR_EN;
> + DEBUG ((DEBUG_INFO, "Enabled V4 64 bit system bus support\n"));
> + }
> }
> //
> // Check if controller version V4.10 or higher
> //
> - if (ControllerVer >= SD_MMC_HC_CTRL_VER_410) {
> - HostCtrl2 |= SD_MMC_HC_26_DATA_LEN_ADMA_EN;
> - DEBUG ((DEBUG_INFO, "Enabled V4 26 bit data length ADMA
> support\n"));
> + else if (ControllerVer >= SD_MMC_HC_CTRL_VER_410) {
> + //
> + // Check if 64bit support is available
> + //
> + if (Capability.SysBus64V4 != 0) {
> + HostCtrl2 |= SD_MMC_HC_64_ADDR_EN;
> + DEBUG ((DEBUG_INFO, "Enabled V4 64 bit system bus support\n"));
> + }
> + if (ControllerVer >= SD_MMC_HC_CTRL_VER_410) {
I think the above 'if' statement can be removed.
> + HostCtrl2 |= SD_MMC_HC_26_DATA_LEN_ADMA_EN;
> + DEBUG ((DEBUG_INFO, "Enabled V4 26 bit data length ADMA
> support\n"));
> + }
> }
> Status = SdMmcHcOrMmio (PciIo, Slot, SD_MMC_HC_HOST_CTRL2, sizeof
> (HostCtrl2), &HostCtrl2);
> if (EFI_ERROR (Status)) {
> @@ -1393,14 +1407,10 @@ BuildAdmaDescTable (
> EFI_PCI_IO_PROTOCOL *PciIo;
> EFI_STATUS Status;
> UINTN Bytes;
> - BOOLEAN AddressingMode64;
> - BOOLEAN DataLength26;
> UINT32 AdmaMaxDataPerLine;
> UINT32 DescSize;
> VOID *AdmaDesc;
>
> - AddressingMode64 = FALSE;
> - DataLength26 = FALSE;
> AdmaMaxDataPerLine = ADMA_MAX_DATA_PER_LINE_16B;
> DescSize = sizeof (SD_MMC_HC_ADMA_32_DESC_LINE);
> AdmaDesc = NULL;
> @@ -1410,27 +1420,16 @@ BuildAdmaDescTable (
> PciIo = Trb->Private->PciIo;
>
> //
> - // Detect whether 64bit addressing is supported.
> - //
> - if (ControllerVer >= SD_MMC_HC_CTRL_VER_400) {
> - Status = SdMmcHcCheckMmioSet(PciIo, Trb->Slot,
> SD_MMC_HC_HOST_CTRL2, sizeof(UINT16),
> - SD_MMC_HC_V4_EN|SD_MMC_HC_64_ADDR_EN,
> SD_MMC_HC_V4_EN|SD_MMC_HC_64_ADDR_EN);
> - if (!EFI_ERROR (Status)) {
> - AddressingMode64 = TRUE;
> - DescSize = sizeof (SD_MMC_HC_ADMA_64_DESC_LINE);
> - }
> - }
> - //
> // Check for valid ranges in 32bit ADMA Descriptor Table
> //
> - if (!AddressingMode64 &&
> + if ((Trb->Mode == SdMmcAdma32bMode) &&
> ((Data >= 0x100000000ul) || ((Data + DataLen) > 0x100000000ul))) {
> return EFI_INVALID_PARAMETER;
> }
> //
> // Check address field alignment
> //
> - if (AddressingMode64) {
> + if (Trb->Mode != SdMmcAdma32bMode) {
> //
> // Address field shall be set on 64-bit boundary (Lower 3-bit is always set
> to 0)
> //
> @@ -1445,13 +1444,19 @@ BuildAdmaDescTable (
> DEBUG ((DEBUG_INFO, "The buffer [0x%x] to construct ADMA desc is not
> aligned to 4 bytes boundary!\n", Data));
> }
> }
> +
> //
> - // Detect whether 26bit data length is supported.
> + // Configure 64b ADMA.
> //
> - Status = SdMmcHcCheckMmioSet(PciIo, Trb->Slot,
> SD_MMC_HC_HOST_CTRL2, sizeof(UINT16),
> - SD_MMC_HC_26_DATA_LEN_ADMA_EN,
> SD_MMC_HC_26_DATA_LEN_ADMA_EN);
> - if (!EFI_ERROR (Status)) {
> - DataLength26 = TRUE;
> + if (Trb->Mode == SdMmcAdma64bV3Mode) {
> + DescSize = sizeof (SD_MMC_HC_ADMA_64_V3_DESC_LINE);
> + }else if (Trb->Mode == SdMmcAdma64bV4Mode) {
> + DescSize = sizeof (SD_MMC_HC_ADMA_64_V4_DESC_LINE);
> + }
> + //
> + // Configure 26b data length.
> + //
> + if (Trb->Length == SdMmcAdmaLen26b) {
> AdmaMaxDataPerLine = ADMA_MAX_DATA_PER_LINE_26B;
> }
>
> @@ -1492,7 +1497,7 @@ BuildAdmaDescTable (
> return EFI_OUT_OF_RESOURCES;
> }
>
> - if ((!AddressingMode64) &&
> + if ((Trb->Mode == SdMmcAdma32bMode) &&
> (UINT64)(UINTN)Trb->AdmaDescPhy > 0x100000000ul) {
> //
> // The ADMA doesn't support 64bit addressing.
> @@ -1511,19 +1516,26 @@ BuildAdmaDescTable (
>
> Remaining = DataLen;
> Address = Data;
> - if (!AddressingMode64) {
> + if (Trb->Mode == SdMmcAdma32bMode) {
> Trb->Adma32Desc = AdmaDesc;
> - Trb->Adma64Desc = NULL;
> + Trb->Adma64V3Desc = NULL;
> + Trb->Adma64V4Desc = NULL;
> + } else if (Trb->Mode == SdMmcAdma64bV3Mode) {
> + Trb->Adma64V3Desc = AdmaDesc;
> + Trb->Adma32Desc = NULL;
> + Trb->Adma64V4Desc = NULL;
> } else {
> - Trb->Adma64Desc = AdmaDesc;
> + Trb->Adma64V4Desc = AdmaDesc;
> Trb->Adma32Desc = NULL;
> + Trb->Adma64V3Desc = NULL;
> }
Suggest to remove those NULL assignments to:
Trb->Adma32Desc
Trb->Adma64Desc
Trb->Adma64V3Desc
Since in SdMmcCreateTrb(), 'Trb' is allocated by:
Trb = AllocateZeroPool (sizeof (SD_MMC_HC_TRB));
> +
> for (Index = 0; Index < Entries; Index++) {
> - if (!AddressingMode64) {
> + if (Trb->Mode == SdMmcAdma32bMode) {
> if (Remaining <= AdmaMaxDataPerLine) {
> Trb->Adma32Desc[Index].Valid = 1;
> Trb->Adma32Desc[Index].Act = 2;
> - if (DataLength26) {
> + if (Trb->Length == SdMmcAdmaLen26b) {
> Trb->Adma32Desc[Index].UpperLength = (UINT16)RShiftU64
> (Remaining, 16);
> }
> Trb->Adma32Desc[Index].LowerLength = (UINT16)(Remaining &
> MAX_UINT16);
> @@ -1532,32 +1544,53 @@ BuildAdmaDescTable (
> } else {
> Trb->Adma32Desc[Index].Valid = 1;
> Trb->Adma32Desc[Index].Act = 2;
> - if (DataLength26) {
> + if (Trb->Length == SdMmcAdmaLen26b) {
> Trb->Adma32Desc[Index].UpperLength = 0;
> }
> Trb->Adma32Desc[Index].LowerLength = 0;
> Trb->Adma32Desc[Index].Address = (UINT32)Address;
> }
> + } else if (Trb->Mode == SdMmcAdma64bV3Mode) {
> + if (Remaining <= AdmaMaxDataPerLine) {
> + Trb->Adma64V3Desc[Index].Valid = 1;
> + Trb->Adma64V3Desc[Index].Act = 2;
> + if (Trb->Length == SdMmcAdmaLen26b) {
> + Trb->Adma64V3Desc[Index].UpperLength = (UINT16)RShiftU64
> (Remaining, 16);
> + }
> + Trb->Adma64V3Desc[Index].LowerLength = (UINT16)(Remaining &
> MAX_UINT16);
> + Trb->Adma64V3Desc[Index].LowerAddress = (UINT32)Address;
> + Trb->Adma64V3Desc[Index].UpperAddress = (UINT32)RShiftU64
> (Address, 32);
> + break;
> + } else {
> + Trb->Adma64V3Desc[Index].Valid = 1;
> + Trb->Adma64V3Desc[Index].Act = 2;
> + if (Trb->Length == SdMmcAdmaLen26b) {
> + Trb->Adma64V3Desc[Index].UpperLength = 0;
> + }
> + Trb->Adma64V3Desc[Index].LowerLength = 0;
> + Trb->Adma64V3Desc[Index].LowerAddress = (UINT32)Address;
> + Trb->Adma64V3Desc[Index].UpperAddress = (UINT32)RShiftU64
> (Address, 32);
> + }
> } else {
> if (Remaining <= AdmaMaxDataPerLine) {
> - Trb->Adma64Desc[Index].Valid = 1;
> - Trb->Adma64Desc[Index].Act = 2;
> - if (DataLength26) {
> - Trb->Adma64Desc[Index].UpperLength = (UINT16)RShiftU64
> (Remaining, 16);
> + Trb->Adma64V4Desc[Index].Valid = 1;
> + Trb->Adma64V4Desc[Index].Act = 2;
> + if (Trb->Length == SdMmcAdmaLen26b) {
> + Trb->Adma64V4Desc[Index].UpperLength = (UINT16)RShiftU64
> (Remaining, 16);
> }
> - Trb->Adma64Desc[Index].LowerLength = (UINT16)(Remaining &
> MAX_UINT16);
> - Trb->Adma64Desc[Index].LowerAddress = (UINT32)Address;
> - Trb->Adma64Desc[Index].UpperAddress = (UINT32)RShiftU64 (Address,
> 32);
> + Trb->Adma64V4Desc[Index].LowerLength = (UINT16)(Remaining &
> MAX_UINT16);
> + Trb->Adma64V4Desc[Index].LowerAddress = (UINT32)Address;
> + Trb->Adma64V4Desc[Index].UpperAddress = (UINT32)RShiftU64
> (Address, 32);
> break;
> } else {
> - Trb->Adma64Desc[Index].Valid = 1;
> - Trb->Adma64Desc[Index].Act = 2;
> - if (DataLength26) {
> - Trb->Adma64Desc[Index].UpperLength = 0;
> + Trb->Adma64V4Desc[Index].Valid = 1;
> + Trb->Adma64V4Desc[Index].Act = 2;
> + if (Trb->Length == SdMmcAdmaLen26b) {
> + Trb->Adma64V4Desc[Index].UpperLength = 0;
> }
> - Trb->Adma64Desc[Index].LowerLength = 0;
> - Trb->Adma64Desc[Index].LowerAddress = (UINT32)Address;
> - Trb->Adma64Desc[Index].UpperAddress = (UINT32)RShiftU64 (Address,
> 32);
> + Trb->Adma64V4Desc[Index].LowerLength = 0;
> + Trb->Adma64V4Desc[Index].LowerAddress = (UINT32)Address;
> + Trb->Adma64V4Desc[Index].UpperAddress = (UINT32)RShiftU64
> (Address, 32);
> }
> }
>
> @@ -1568,7 +1601,13 @@ BuildAdmaDescTable (
> //
> // Set the last descriptor line as end of descriptor table
> //
> - AddressingMode64 ? (Trb->Adma64Desc[Index].End = 1) : (Trb-
> >Adma32Desc[Index].End = 1);
> + if (Trb->Mode == SdMmcAdma32bMode) {
> + Trb->Adma32Desc[Index].End = 1;
> + } else if (Trb->Mode == SdMmcAdma64bV3Mode) {
> + Trb->Adma64V3Desc[Index].End = 1;
> + } else {
> + Trb->Adma64V4Desc[Index].End = 1;
> + }
> return EFI_SUCCESS;
> }
>
> @@ -1665,7 +1704,20 @@ SdMmcCreateTrb (
> if (Trb->DataLen == 0) {
> Trb->Mode = SdMmcNoData;
> } else if (Private->Capability[Slot].Adma2 != 0) {
> - Trb->Mode = SdMmcAdmaMode;
> + Trb->Mode = SdMmcAdma32bMode;
> + Trb->Length = SdMmcAdmaLen16b;
> + if ((Private->ControllerVersion[Slot] == SD_MMC_HC_CTRL_VER_300)
> &&
> + (Private->Capability[Slot].SysBus64V3 == 1)) {
> + Trb->Mode = SdMmcAdma64bV3Mode;
> + } else if (((Private->ControllerVersion[Slot] ==
> SD_MMC_HC_CTRL_VER_400) &&
> + (Private->Capability[Slot].SysBus64V3 == 1)) ||
> + ((Private->ControllerVersion[Slot] >= SD_MMC_HC_CTRL_VER_410)
> &&
> + (Private->Capability[Slot].SysBus64V4 == 1))) {
> + Trb->Mode = SdMmcAdma64bV4Mode;
> + }
> + if (Private->ControllerVersion[Slot] >= SD_MMC_HC_CTRL_VER_410) {
> + Trb->Length = SdMmcAdmaLen26b;
> + }
> Status = BuildAdmaDescTable (Trb, Private->ControllerVersion[Slot]);
> if (EFI_ERROR (Status)) {
> PciIo->Unmap (PciIo, Trb->DataMap);
> @@ -1719,11 +1771,18 @@ SdMmcFreeTrb (
> Trb->Adma32Desc
> );
> }
> - if (Trb->Adma64Desc != NULL) {
> + if (Trb->Adma64V3Desc != NULL) {
> + PciIo->FreeBuffer (
> + PciIo,
> + Trb->AdmaPages,
> + Trb->Adma64V3Desc
> + );
> + }
> + if (Trb->Adma64V4Desc != NULL) {
> PciIo->FreeBuffer (
> PciIo,
> Trb->AdmaPages,
> - Trb->Adma64Desc
> + Trb->Adma64V4Desc
> );
> }
> if (Trb->DataMap != NULL) {
> @@ -1891,27 +1950,35 @@ SdMmcExecTrb (
> if (EFI_ERROR (Status)) {
> return Status;
> }
> +
> + if (Private->ControllerVersion[Trb->Slot] >= SD_MMC_HC_CTRL_VER_400)
> {
> + Status = SdMmcHcCheckMmioSet(PciIo, Trb->Slot,
> SD_MMC_HC_HOST_CTRL2, sizeof(UINT16),
> + SD_MMC_HC_64_ADDR_EN, SD_MMC_HC_64_ADDR_EN);
> + if (!EFI_ERROR (Status)) {
> + AddressingMode64 = TRUE;
> + }
> + }
> +
> //
> // Set Host Control 1 register DMA Select field
> //
> - if (Trb->Mode == SdMmcAdmaMode) {
> + if ((Trb->Mode == SdMmcAdma32bMode) ||
> + (Trb->Mode == SdMmcAdma64bV4Mode)) {
> HostCtrl1 = BIT4;
> Status = SdMmcHcOrMmio (PciIo, Trb->Slot, SD_MMC_HC_HOST_CTRL1,
> sizeof (HostCtrl1), &HostCtrl1);
> if (EFI_ERROR (Status)) {
> return Status;
> }
> + } else if (Trb->Mode == SdMmcAdma64bV3Mode) {
> + HostCtrl1 = BIT4|BIT3;
> + Status = SdMmcHcOrMmio (PciIo, Trb->Slot, SD_MMC_HC_HOST_CTRL1,
> sizeof (HostCtrl1), &HostCtrl1);
> + if (EFI_ERROR (Status)) {
> + return Status;
> + }
> }
>
> SdMmcHcLedOnOff (PciIo, Trb->Slot, TRUE);
>
> - if (Private->ControllerVersion[Trb->Slot] >= SD_MMC_HC_CTRL_VER_400)
> {
> - Status = SdMmcHcCheckMmioSet(PciIo, Trb->Slot,
> SD_MMC_HC_HOST_CTRL2, sizeof(UINT16),
> - SD_MMC_HC_V4_EN|SD_MMC_HC_64_ADDR_EN,
> SD_MMC_HC_V4_EN|SD_MMC_HC_64_ADDR_EN);
> - if (!EFI_ERROR (Status)) {
> - AddressingMode64 = TRUE;
> - }
> - }
> -
> if (Trb->Mode == SdMmcSdmaMode) {
> if ((!AddressingMode64) &&
> ((UINT64)(UINTN)Trb->DataPhy >= 0x100000000ul)) {
> @@ -1929,7 +1996,9 @@ SdMmcExecTrb (
> if (EFI_ERROR (Status)) {
> return Status;
> }
> - } else if (Trb->Mode == SdMmcAdmaMode) {
> + } else if ((Trb->Mode == SdMmcAdma32bMode) ||
> + (Trb->Mode == SdMmcAdma64bV3Mode) ||
> + (Trb->Mode == SdMmcAdma64bV4Mode)) {
> AdmaAddr = (UINT64)(UINTN)Trb->AdmaDescPhy;
> Status = SdMmcHcRwMmio (PciIo, Trb->Slot,
> SD_MMC_HC_ADMA_SYS_ADDR, FALSE, sizeof (AdmaAddr), &AdmaAddr);
> if (EFI_ERROR (Status)) {
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> index d157f2c..3a05456 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> @@ -2,7 +2,7 @@
>
> Provides some data structure definitions used by the SD/MMC host
> controller driver.
>
> -Copyright (c) 2018, NVIDIA CORPORATION. All rights reserved.
> +Copyright (c) 2018-2019, NVIDIA CORPORATION. All rights reserved.
> Copyright (c) 2015, Intel Corporation. All rights reserved.<BR>
> This program and the accompanying materials
> are licensed and made available under the terms and conditions of the BSD
> License
> @@ -80,16 +80,25 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF
> ANY KIND, EITHER EXPRESS OR IMPLIED.
>
> //
> // The transfer modes supported by SD Host Controller
> -// Simplified Spec 3.0 Table 1-2
> //
> typedef enum {
> SdMmcNoData,
> SdMmcPioMode,
> SdMmcSdmaMode,
> - SdMmcAdmaMode
> + SdMmcAdma32bMode,
> + SdMmcAdma64bV3Mode,
> + SdMmcAdma64bV4Mode
> } SD_MMC_HC_TRANSFER_MODE;
>
> //
> +// The ADMA transfer lengths supported by SD Host Controller
> +//
> +typedef enum {
> + SdMmcAdmaLen16b,
> + SdMmcAdmaLen26b
> +} SD_MMC_HC_ADMA_LEGTH;
Typo 'LEGTH' -> 'LENGTH'
Also, how about 'SD_MMC_HC_ADMA_LENGTH_MODE'?
Best Regards,
Hao Wu
> +
> +//
> // The maximum data length of each descriptor line
> //
> #define ADMA_MAX_DATA_PER_LINE_16B SIZE_64KB
> @@ -122,8 +131,20 @@ typedef struct {
> UINT32 LowerLength:16;
> UINT32 LowerAddress;
> UINT32 UpperAddress;
> +} SD_MMC_HC_ADMA_64_V3_DESC_LINE;
> +
> +typedef struct {
> + UINT32 Valid:1;
> + UINT32 End:1;
> + UINT32 Int:1;
> + UINT32 Reserved:1;
> + UINT32 Act:2;
> + UINT32 UpperLength:10;
> + UINT32 LowerLength:16;
> + UINT32 LowerAddress;
> + UINT32 UpperAddress;
> UINT32 Reserved1;
> -} SD_MMC_HC_ADMA_64_DESC_LINE;
> +} SD_MMC_HC_ADMA_64_V4_DESC_LINE;
>
> #define SD_MMC_SDMA_BOUNDARY 512 * 1024
> #define SD_MMC_SDMA_ROUND_UP(x, n) (((x) + n) & ~(n - 1))
> --
> 2.7.4
________________________________
This email message is for the sole use of the intended recipient(s) and may contain confidential information. Any unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply email and destroy all copies of the original message.
________________________________
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] MdeModulePkg/SdMmcPciHcDxe: Add V3 64b DMA Support
2019-03-07 0:54 ` Cohen, Eugene
@ 2019-03-07 1:34 ` Wu, Hao A
0 siblings, 0 replies; 9+ messages in thread
From: Wu, Hao A @ 2019-03-07 1:34 UTC (permalink / raw)
To: Cohen, Eugene, Ashish Singhal, edk2-devel@lists.01.org
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Cohen, Eugene
> Sent: Thursday, March 07, 2019 8:54 AM
> To: Ashish Singhal; Wu, Hao A; edk2-devel@lists.01.org
> Subject: Re: [edk2] [PATCH] MdeModulePkg/SdMmcPciHcDxe: Add V3 64b
> DMA Support
>
> Tested-by: Eugene Cohen <eugene@hp.com>
Eugene,
Thanks for the testing. Really appreciate your help.
Best Regards,
Hao Wu
>
> Thanks again.
>
> From: Ashish Singhal <ashishsingha@nvidia.com>
> Sent: Wednesday, March 6, 2019 4:07 PM
> To: Cohen, Eugene <eugene@hp.com>; Wu, Hao A <hao.a.wu@intel.com>;
> edk2-devel@lists.01.org
> Subject: RE: [PATCH] MdeModulePkg/SdMmcPciHcDxe: Add V3 64b DMA
> Support
>
> Hi Eugene,
>
> Thanks for confirming. Can you please validate the v2 patch I sent as well for
> completeness?
>
> Thanks
> Ashish
>
> From: Cohen, Eugene <eugene@hp.com<mailto:eugene@hp.com>>
> Sent: Wednesday, March 6, 2019 4:05 PM
> To: Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>; Ashish
> Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com>>;
> edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
> Subject: RE: [PATCH] MdeModulePkg/SdMmcPciHcDxe: Add V3 64b DMA
> Support
>
>
> Ø I verified the patch on SDHC version 3.00 with 64-bit System Address
> Ø Support. Hope more configurations are available for testing on Eugene's
> Ø side.
>
> This patch works for us. Tested that the V3 64-bit DMA works and verified
> that addresses above 4GB DMA correctly.
>
> Thanks for putting this together.
>
> Feel free to add my Tested-By.
>
> Eugene
>
> From: Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>
> Sent: Tuesday, March 5, 2019 8:01 PM
> To: Ashish Singhal
> <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com>>; edk2-
> devel@lists.01.org<mailto:edk2-devel@lists.01.org>
> Cc: Cohen, Eugene <eugene@hp.com<mailto:eugene@hp.com>>
> Subject: RE: [PATCH] MdeModulePkg/SdMmcPciHcDxe: Add V3 64b DMA
> Support
>
> Hi Ashish,
>
> One thing to confirm, for the updated checks within
> SdMmcPciHcDriverBindingStart():
>
> > if ((Private->ControllerVersion[Slot] == SD_MMC_HC_CTRL_VER_300 &&
> > Private->Capability[Slot].SysBus64V3 == 0) ||
> > (Private->ControllerVersion[Slot] == SD_MMC_HC_CTRL_VER_400 &&
> > Private->Capability[Slot].SysBus64V3 == 0) ||
> > (Private->ControllerVersion[Slot] >= SD_MMC_HC_CTRL_VER_410 &&
> > Private->Capability[Slot].SysBus64V4 == 0)) {
> > Support64BitDma = FALSE;
> > }
>
> When the SDHC with version greater than 4.10, the check is only performed
> against the 'SysBus64V4' bit. My understanding of the purpose is that:
>
> 1. For SDHC with version 4.00, the support of V3 mode and V4 mode of
> 64-bit System Address are reflect by bit 'SysBus64V3'. Thus, I can infer
> that the possible support case is both or neither.
>
> 2. The spec states that SDHC with version greater than 4.10 divides the V3
> mode and V4 mode support into 2 bits (SysBus64V3, SysBus64V4) so that the
> V3 mode support can be optional.
>
> So based on 1 & 2, we do not even bother to check the 'SysBus64V3' bit
> when HC version >= 4.10. Is that right?
>
>
> I verified the patch on SDHC version 3.00 with 64-bit System Address
> Support. Hope more configurations are available for testing on Eugene's
> side.
>
>
> Besides, some minor comments below:
>
> > -----Original Message-----
> > From: Ashish Singhal [mailto:ashishsingha@nvidia.com]
> > Sent: Saturday, March 02, 2019 2:30 AM
> > To: edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
> > Cc: Wu, Hao A; eugene@hp.com<mailto:eugene@hp.com>; Ashish Singhal
> > Subject: [PATCH] MdeModulePkg/SdMmcPciHcDxe: Add V3 64b DMA
> > Support
>
> Please help to add the below Bugzilla tracker for reference:
> https://bugzilla.tianocore.org/show_bug.cgi?id=1583<https://bugzilla.tianoc
> ore.org/show_bug.cgi?id=1583>
>
> I have updated the above tracker to match the purpose of the proposed
> patch.
>
>
> >
> > Driver was supporting only 32b DMA support for V3 controllers. Add
> > support for 64b DMA as well for completeness.
> >
> > For V4.0 64b support, driver was looking at incorrect capability
> > register bit. Fix for that is present as well.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Ashish Singhal
> <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com>>
> > ---
> > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c | 10 +-
> > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h | 6 +-
> > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c | 199
> > ++++++++++++++-------
> > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h | 29 ++-
> > 4 files changed, 170 insertions(+), 74 deletions(-)
> >
> > diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> > b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> > index b474f8d..9b7b88c 100644
> > --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> > +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> > @@ -6,7 +6,7 @@
> >
> > It would expose EFI_SD_MMC_PASS_THRU_PROTOCOL for upper layer
> use.
> >
> > - Copyright (c) 2018, NVIDIA CORPORATION. All rights reserved.
> > + Copyright (c) 2018-2019, NVIDIA CORPORATION. All rights reserved.
> > Copyright (c) 2015 - 2019, Intel Corporation. All rights reserved.<BR>
> > This program and the accompanying materials
> > are licensed and made available under the terms and conditions of the BSD
> > License
> > @@ -666,8 +666,12 @@ SdMmcPciHcDriverBindingStart (
> > // If any of the slots does not support 64b system bus
> > // do not enable 64b DMA in the PCI layer.
> > //
> > - if (Private->Capability[Slot].SysBus64V3 == 0 &&
> > - Private->Capability[Slot].SysBus64V4 == 0) {
> > + if ((Private->ControllerVersion[Slot] == SD_MMC_HC_CTRL_VER_300 &&
> > + Private->Capability[Slot].SysBus64V3 == 0) ||
> > + (Private->ControllerVersion[Slot] == SD_MMC_HC_CTRL_VER_400 &&
> > + Private->Capability[Slot].SysBus64V3 == 0) ||
> > + (Private->ControllerVersion[Slot] >= SD_MMC_HC_CTRL_VER_410 &&
> > + Private->Capability[Slot].SysBus64V4 == 0)) {
> > Support64BitDma = FALSE;
> > }
> >
> > diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> > b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> > index 1bb701a..68d8a5c 100644
> > --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> > +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> > @@ -2,7 +2,7 @@
> >
> > Provides some data structure definitions used by the SD/MMC host
> > controller driver.
> >
> > -Copyright (c) 2018, NVIDIA CORPORATION. All rights reserved.
> > +Copyright (c) 2018-2019, NVIDIA CORPORATION. All rights reserved.
> > Copyright (c) 2015, Intel Corporation. All rights reserved.<BR>
> > This program and the accompanying materials
> > are licensed and made available under the terms and conditions of the BSD
> > License
> > @@ -145,13 +145,15 @@ typedef struct {
> > EFI_PHYSICAL_ADDRESS DataPhy;
> > VOID *DataMap;
> > SD_MMC_HC_TRANSFER_MODE Mode;
> > + SD_MMC_HC_ADMA_LEGTH Length;
>
> Maybe:
> SD_MMC_HC_ADMA_LENGTH_MODE AdmaLengthMode;
>
> is better to avoid confusion.
>
>
> >
> > EFI_EVENT Event;
> > BOOLEAN Started;
> > UINT64 Timeout;
> >
> > SD_MMC_HC_ADMA_32_DESC_LINE *Adma32Desc;
> > - SD_MMC_HC_ADMA_64_DESC_LINE *Adma64Desc;
> > + SD_MMC_HC_ADMA_64_V3_DESC_LINE *Adma64V3Desc;
> > + SD_MMC_HC_ADMA_64_V4_DESC_LINE *Adma64V4Desc;
> > EFI_PHYSICAL_ADDRESS AdmaDescPhy;
> > VOID *AdmaMap;
> > UINT32 AdmaPages;
> > diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> > b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> > index d73fa10..a6d2395 100644
> > --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> > +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> > @@ -6,7 +6,7 @@
> >
> > It would expose EFI_SD_MMC_PASS_THRU_PROTOCOL for upper layer
> use.
> >
> > - Copyright (c) 2018, NVIDIA CORPORATION. All rights reserved.
> > + Copyright (c) 2018-2019, NVIDIA CORPORATION. All rights reserved.
> > Copyright (c) 2015 - 2019, Intel Corporation. All rights reserved.<BR>
> > This program and the accompanying materials
> > are licensed and made available under the terms and conditions of the BSD
> > License
> > @@ -1010,18 +1010,32 @@ SdMmcHcInitV4Enhancements (
> > if (ControllerVer >= SD_MMC_HC_CTRL_VER_400) {
> > HostCtrl2 = SD_MMC_HC_V4_EN;
> > //
> > - // Check if V4 64bit support is available
> > + // Check if controller version V4.0
> > //
> > - if (Capability.SysBus64V4 != 0) {
> > - HostCtrl2 |= SD_MMC_HC_64_ADDR_EN;
> > - DEBUG ((DEBUG_INFO, "Enabled V4 64 bit system bus support\n"));
> > + if (ControllerVer == SD_MMC_HC_CTRL_VER_400) {
> > + //
> > + // Check if 64bit support is available
> > + //
> > + if (Capability.SysBus64V3 != 0) {
> > + HostCtrl2 |= SD_MMC_HC_64_ADDR_EN;
> > + DEBUG ((DEBUG_INFO, "Enabled V4 64 bit system bus support\n"));
> > + }
> > }
> > //
> > // Check if controller version V4.10 or higher
> > //
> > - if (ControllerVer >= SD_MMC_HC_CTRL_VER_410) {
> > - HostCtrl2 |= SD_MMC_HC_26_DATA_LEN_ADMA_EN;
> > - DEBUG ((DEBUG_INFO, "Enabled V4 26 bit data length ADMA
> > support\n"));
> > + else if (ControllerVer >= SD_MMC_HC_CTRL_VER_410) {
> > + //
> > + // Check if 64bit support is available
> > + //
> > + if (Capability.SysBus64V4 != 0) {
> > + HostCtrl2 |= SD_MMC_HC_64_ADDR_EN;
> > + DEBUG ((DEBUG_INFO, "Enabled V4 64 bit system bus support\n"));
> > + }
> > + if (ControllerVer >= SD_MMC_HC_CTRL_VER_410) {
>
> I think the above 'if' statement can be removed.
>
>
> > + HostCtrl2 |= SD_MMC_HC_26_DATA_LEN_ADMA_EN;
> > + DEBUG ((DEBUG_INFO, "Enabled V4 26 bit data length ADMA
> > support\n"));
> > + }
> > }
> > Status = SdMmcHcOrMmio (PciIo, Slot, SD_MMC_HC_HOST_CTRL2, sizeof
> > (HostCtrl2), &HostCtrl2);
> > if (EFI_ERROR (Status)) {
> > @@ -1393,14 +1407,10 @@ BuildAdmaDescTable (
> > EFI_PCI_IO_PROTOCOL *PciIo;
> > EFI_STATUS Status;
> > UINTN Bytes;
> > - BOOLEAN AddressingMode64;
> > - BOOLEAN DataLength26;
> > UINT32 AdmaMaxDataPerLine;
> > UINT32 DescSize;
> > VOID *AdmaDesc;
> >
> > - AddressingMode64 = FALSE;
> > - DataLength26 = FALSE;
> > AdmaMaxDataPerLine = ADMA_MAX_DATA_PER_LINE_16B;
> > DescSize = sizeof (SD_MMC_HC_ADMA_32_DESC_LINE);
> > AdmaDesc = NULL;
> > @@ -1410,27 +1420,16 @@ BuildAdmaDescTable (
> > PciIo = Trb->Private->PciIo;
> >
> > //
> > - // Detect whether 64bit addressing is supported.
> > - //
> > - if (ControllerVer >= SD_MMC_HC_CTRL_VER_400) {
> > - Status = SdMmcHcCheckMmioSet(PciIo, Trb->Slot,
> > SD_MMC_HC_HOST_CTRL2, sizeof(UINT16),
> > - SD_MMC_HC_V4_EN|SD_MMC_HC_64_ADDR_EN,
> > SD_MMC_HC_V4_EN|SD_MMC_HC_64_ADDR_EN);
> > - if (!EFI_ERROR (Status)) {
> > - AddressingMode64 = TRUE;
> > - DescSize = sizeof (SD_MMC_HC_ADMA_64_DESC_LINE);
> > - }
> > - }
> > - //
> > // Check for valid ranges in 32bit ADMA Descriptor Table
> > //
> > - if (!AddressingMode64 &&
> > + if ((Trb->Mode == SdMmcAdma32bMode) &&
> > ((Data >= 0x100000000ul) || ((Data + DataLen) > 0x100000000ul))) {
> > return EFI_INVALID_PARAMETER;
> > }
> > //
> > // Check address field alignment
> > //
> > - if (AddressingMode64) {
> > + if (Trb->Mode != SdMmcAdma32bMode) {
> > //
> > // Address field shall be set on 64-bit boundary (Lower 3-bit is always set
> > to 0)
> > //
> > @@ -1445,13 +1444,19 @@ BuildAdmaDescTable (
> > DEBUG ((DEBUG_INFO, "The buffer [0x%x] to construct ADMA desc is not
> > aligned to 4 bytes boundary!\n", Data));
> > }
> > }
> > +
> > //
> > - // Detect whether 26bit data length is supported.
> > + // Configure 64b ADMA.
> > //
> > - Status = SdMmcHcCheckMmioSet(PciIo, Trb->Slot,
> > SD_MMC_HC_HOST_CTRL2, sizeof(UINT16),
> > - SD_MMC_HC_26_DATA_LEN_ADMA_EN,
> > SD_MMC_HC_26_DATA_LEN_ADMA_EN);
> > - if (!EFI_ERROR (Status)) {
> > - DataLength26 = TRUE;
> > + if (Trb->Mode == SdMmcAdma64bV3Mode) {
> > + DescSize = sizeof (SD_MMC_HC_ADMA_64_V3_DESC_LINE);
> > + }else if (Trb->Mode == SdMmcAdma64bV4Mode) {
> > + DescSize = sizeof (SD_MMC_HC_ADMA_64_V4_DESC_LINE);
> > + }
> > + //
> > + // Configure 26b data length.
> > + //
> > + if (Trb->Length == SdMmcAdmaLen26b) {
> > AdmaMaxDataPerLine = ADMA_MAX_DATA_PER_LINE_26B;
> > }
> >
> > @@ -1492,7 +1497,7 @@ BuildAdmaDescTable (
> > return EFI_OUT_OF_RESOURCES;
> > }
> >
> > - if ((!AddressingMode64) &&
> > + if ((Trb->Mode == SdMmcAdma32bMode) &&
> > (UINT64)(UINTN)Trb->AdmaDescPhy > 0x100000000ul) {
> > //
> > // The ADMA doesn't support 64bit addressing.
> > @@ -1511,19 +1516,26 @@ BuildAdmaDescTable (
> >
> > Remaining = DataLen;
> > Address = Data;
> > - if (!AddressingMode64) {
> > + if (Trb->Mode == SdMmcAdma32bMode) {
> > Trb->Adma32Desc = AdmaDesc;
> > - Trb->Adma64Desc = NULL;
> > + Trb->Adma64V3Desc = NULL;
> > + Trb->Adma64V4Desc = NULL;
> > + } else if (Trb->Mode == SdMmcAdma64bV3Mode) {
> > + Trb->Adma64V3Desc = AdmaDesc;
> > + Trb->Adma32Desc = NULL;
> > + Trb->Adma64V4Desc = NULL;
> > } else {
> > - Trb->Adma64Desc = AdmaDesc;
> > + Trb->Adma64V4Desc = AdmaDesc;
> > Trb->Adma32Desc = NULL;
> > + Trb->Adma64V3Desc = NULL;
> > }
>
> Suggest to remove those NULL assignments to:
> Trb->Adma32Desc
> Trb->Adma64Desc
> Trb->Adma64V3Desc
>
> Since in SdMmcCreateTrb(), 'Trb' is allocated by:
>
> Trb = AllocateZeroPool (sizeof (SD_MMC_HC_TRB));
>
>
> > +
> > for (Index = 0; Index < Entries; Index++) {
> > - if (!AddressingMode64) {
> > + if (Trb->Mode == SdMmcAdma32bMode) {
> > if (Remaining <= AdmaMaxDataPerLine) {
> > Trb->Adma32Desc[Index].Valid = 1;
> > Trb->Adma32Desc[Index].Act = 2;
> > - if (DataLength26) {
> > + if (Trb->Length == SdMmcAdmaLen26b) {
> > Trb->Adma32Desc[Index].UpperLength = (UINT16)RShiftU64
> > (Remaining, 16);
> > }
> > Trb->Adma32Desc[Index].LowerLength = (UINT16)(Remaining &
> > MAX_UINT16);
> > @@ -1532,32 +1544,53 @@ BuildAdmaDescTable (
> > } else {
> > Trb->Adma32Desc[Index].Valid = 1;
> > Trb->Adma32Desc[Index].Act = 2;
> > - if (DataLength26) {
> > + if (Trb->Length == SdMmcAdmaLen26b) {
> > Trb->Adma32Desc[Index].UpperLength = 0;
> > }
> > Trb->Adma32Desc[Index].LowerLength = 0;
> > Trb->Adma32Desc[Index].Address = (UINT32)Address;
> > }
> > + } else if (Trb->Mode == SdMmcAdma64bV3Mode) {
> > + if (Remaining <= AdmaMaxDataPerLine) {
> > + Trb->Adma64V3Desc[Index].Valid = 1;
> > + Trb->Adma64V3Desc[Index].Act = 2;
> > + if (Trb->Length == SdMmcAdmaLen26b) {
> > + Trb->Adma64V3Desc[Index].UpperLength = (UINT16)RShiftU64
> > (Remaining, 16);
> > + }
> > + Trb->Adma64V3Desc[Index].LowerLength = (UINT16)(Remaining &
> > MAX_UINT16);
> > + Trb->Adma64V3Desc[Index].LowerAddress = (UINT32)Address;
> > + Trb->Adma64V3Desc[Index].UpperAddress = (UINT32)RShiftU64
> > (Address, 32);
> > + break;
> > + } else {
> > + Trb->Adma64V3Desc[Index].Valid = 1;
> > + Trb->Adma64V3Desc[Index].Act = 2;
> > + if (Trb->Length == SdMmcAdmaLen26b) {
> > + Trb->Adma64V3Desc[Index].UpperLength = 0;
> > + }
> > + Trb->Adma64V3Desc[Index].LowerLength = 0;
> > + Trb->Adma64V3Desc[Index].LowerAddress = (UINT32)Address;
> > + Trb->Adma64V3Desc[Index].UpperAddress = (UINT32)RShiftU64
> > (Address, 32);
> > + }
> > } else {
> > if (Remaining <= AdmaMaxDataPerLine) {
> > - Trb->Adma64Desc[Index].Valid = 1;
> > - Trb->Adma64Desc[Index].Act = 2;
> > - if (DataLength26) {
> > - Trb->Adma64Desc[Index].UpperLength = (UINT16)RShiftU64
> > (Remaining, 16);
> > + Trb->Adma64V4Desc[Index].Valid = 1;
> > + Trb->Adma64V4Desc[Index].Act = 2;
> > + if (Trb->Length == SdMmcAdmaLen26b) {
> > + Trb->Adma64V4Desc[Index].UpperLength = (UINT16)RShiftU64
> > (Remaining, 16);
> > }
> > - Trb->Adma64Desc[Index].LowerLength = (UINT16)(Remaining &
> > MAX_UINT16);
> > - Trb->Adma64Desc[Index].LowerAddress = (UINT32)Address;
> > - Trb->Adma64Desc[Index].UpperAddress = (UINT32)RShiftU64 (Address,
> > 32);
> > + Trb->Adma64V4Desc[Index].LowerLength = (UINT16)(Remaining &
> > MAX_UINT16);
> > + Trb->Adma64V4Desc[Index].LowerAddress = (UINT32)Address;
> > + Trb->Adma64V4Desc[Index].UpperAddress = (UINT32)RShiftU64
> > (Address, 32);
> > break;
> > } else {
> > - Trb->Adma64Desc[Index].Valid = 1;
> > - Trb->Adma64Desc[Index].Act = 2;
> > - if (DataLength26) {
> > - Trb->Adma64Desc[Index].UpperLength = 0;
> > + Trb->Adma64V4Desc[Index].Valid = 1;
> > + Trb->Adma64V4Desc[Index].Act = 2;
> > + if (Trb->Length == SdMmcAdmaLen26b) {
> > + Trb->Adma64V4Desc[Index].UpperLength = 0;
> > }
> > - Trb->Adma64Desc[Index].LowerLength = 0;
> > - Trb->Adma64Desc[Index].LowerAddress = (UINT32)Address;
> > - Trb->Adma64Desc[Index].UpperAddress = (UINT32)RShiftU64 (Address,
> > 32);
> > + Trb->Adma64V4Desc[Index].LowerLength = 0;
> > + Trb->Adma64V4Desc[Index].LowerAddress = (UINT32)Address;
> > + Trb->Adma64V4Desc[Index].UpperAddress = (UINT32)RShiftU64
> > (Address, 32);
> > }
> > }
> >
> > @@ -1568,7 +1601,13 @@ BuildAdmaDescTable (
> > //
> > // Set the last descriptor line as end of descriptor table
> > //
> > - AddressingMode64 ? (Trb->Adma64Desc[Index].End = 1) : (Trb-
> > >Adma32Desc[Index].End = 1);
> > + if (Trb->Mode == SdMmcAdma32bMode) {
> > + Trb->Adma32Desc[Index].End = 1;
> > + } else if (Trb->Mode == SdMmcAdma64bV3Mode) {
> > + Trb->Adma64V3Desc[Index].End = 1;
> > + } else {
> > + Trb->Adma64V4Desc[Index].End = 1;
> > + }
> > return EFI_SUCCESS;
> > }
> >
> > @@ -1665,7 +1704,20 @@ SdMmcCreateTrb (
> > if (Trb->DataLen == 0) {
> > Trb->Mode = SdMmcNoData;
> > } else if (Private->Capability[Slot].Adma2 != 0) {
> > - Trb->Mode = SdMmcAdmaMode;
> > + Trb->Mode = SdMmcAdma32bMode;
> > + Trb->Length = SdMmcAdmaLen16b;
> > + if ((Private->ControllerVersion[Slot] == SD_MMC_HC_CTRL_VER_300)
> > &&
> > + (Private->Capability[Slot].SysBus64V3 == 1)) {
> > + Trb->Mode = SdMmcAdma64bV3Mode;
> > + } else if (((Private->ControllerVersion[Slot] ==
> > SD_MMC_HC_CTRL_VER_400) &&
> > + (Private->Capability[Slot].SysBus64V3 == 1)) ||
> > + ((Private->ControllerVersion[Slot] >= SD_MMC_HC_CTRL_VER_410)
> > &&
> > + (Private->Capability[Slot].SysBus64V4 == 1))) {
> > + Trb->Mode = SdMmcAdma64bV4Mode;
> > + }
> > + if (Private->ControllerVersion[Slot] >= SD_MMC_HC_CTRL_VER_410) {
> > + Trb->Length = SdMmcAdmaLen26b;
> > + }
> > Status = BuildAdmaDescTable (Trb, Private->ControllerVersion[Slot]);
> > if (EFI_ERROR (Status)) {
> > PciIo->Unmap (PciIo, Trb->DataMap);
> > @@ -1719,11 +1771,18 @@ SdMmcFreeTrb (
> > Trb->Adma32Desc
> > );
> > }
> > - if (Trb->Adma64Desc != NULL) {
> > + if (Trb->Adma64V3Desc != NULL) {
> > + PciIo->FreeBuffer (
> > + PciIo,
> > + Trb->AdmaPages,
> > + Trb->Adma64V3Desc
> > + );
> > + }
> > + if (Trb->Adma64V4Desc != NULL) {
> > PciIo->FreeBuffer (
> > PciIo,
> > Trb->AdmaPages,
> > - Trb->Adma64Desc
> > + Trb->Adma64V4Desc
> > );
> > }
> > if (Trb->DataMap != NULL) {
> > @@ -1891,27 +1950,35 @@ SdMmcExecTrb (
> > if (EFI_ERROR (Status)) {
> > return Status;
> > }
> > +
> > + if (Private->ControllerVersion[Trb->Slot] >=
> SD_MMC_HC_CTRL_VER_400)
> > {
> > + Status = SdMmcHcCheckMmioSet(PciIo, Trb->Slot,
> > SD_MMC_HC_HOST_CTRL2, sizeof(UINT16),
> > + SD_MMC_HC_64_ADDR_EN, SD_MMC_HC_64_ADDR_EN);
> > + if (!EFI_ERROR (Status)) {
> > + AddressingMode64 = TRUE;
> > + }
> > + }
> > +
> > //
> > // Set Host Control 1 register DMA Select field
> > //
> > - if (Trb->Mode == SdMmcAdmaMode) {
> > + if ((Trb->Mode == SdMmcAdma32bMode) ||
> > + (Trb->Mode == SdMmcAdma64bV4Mode)) {
> > HostCtrl1 = BIT4;
> > Status = SdMmcHcOrMmio (PciIo, Trb->Slot, SD_MMC_HC_HOST_CTRL1,
> > sizeof (HostCtrl1), &HostCtrl1);
> > if (EFI_ERROR (Status)) {
> > return Status;
> > }
> > + } else if (Trb->Mode == SdMmcAdma64bV3Mode) {
> > + HostCtrl1 = BIT4|BIT3;
> > + Status = SdMmcHcOrMmio (PciIo, Trb->Slot, SD_MMC_HC_HOST_CTRL1,
> > sizeof (HostCtrl1), &HostCtrl1);
> > + if (EFI_ERROR (Status)) {
> > + return Status;
> > + }
> > }
> >
> > SdMmcHcLedOnOff (PciIo, Trb->Slot, TRUE);
> >
> > - if (Private->ControllerVersion[Trb->Slot] >= SD_MMC_HC_CTRL_VER_400)
> > {
> > - Status = SdMmcHcCheckMmioSet(PciIo, Trb->Slot,
> > SD_MMC_HC_HOST_CTRL2, sizeof(UINT16),
> > - SD_MMC_HC_V4_EN|SD_MMC_HC_64_ADDR_EN,
> > SD_MMC_HC_V4_EN|SD_MMC_HC_64_ADDR_EN);
> > - if (!EFI_ERROR (Status)) {
> > - AddressingMode64 = TRUE;
> > - }
> > - }
> > -
> > if (Trb->Mode == SdMmcSdmaMode) {
> > if ((!AddressingMode64) &&
> > ((UINT64)(UINTN)Trb->DataPhy >= 0x100000000ul)) {
> > @@ -1929,7 +1996,9 @@ SdMmcExecTrb (
> > if (EFI_ERROR (Status)) {
> > return Status;
> > }
> > - } else if (Trb->Mode == SdMmcAdmaMode) {
> > + } else if ((Trb->Mode == SdMmcAdma32bMode) ||
> > + (Trb->Mode == SdMmcAdma64bV3Mode) ||
> > + (Trb->Mode == SdMmcAdma64bV4Mode)) {
> > AdmaAddr = (UINT64)(UINTN)Trb->AdmaDescPhy;
> > Status = SdMmcHcRwMmio (PciIo, Trb->Slot,
> > SD_MMC_HC_ADMA_SYS_ADDR, FALSE, sizeof (AdmaAddr), &AdmaAddr);
> > if (EFI_ERROR (Status)) {
> > diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> > b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> > index d157f2c..3a05456 100644
> > --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> > +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> > @@ -2,7 +2,7 @@
> >
> > Provides some data structure definitions used by the SD/MMC host
> > controller driver.
> >
> > -Copyright (c) 2018, NVIDIA CORPORATION. All rights reserved.
> > +Copyright (c) 2018-2019, NVIDIA CORPORATION. All rights reserved.
> > Copyright (c) 2015, Intel Corporation. All rights reserved.<BR>
> > This program and the accompanying materials
> > are licensed and made available under the terms and conditions of the BSD
> > License
> > @@ -80,16 +80,25 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF
> > ANY KIND, EITHER EXPRESS OR IMPLIED.
> >
> > //
> > // The transfer modes supported by SD Host Controller
> > -// Simplified Spec 3.0 Table 1-2
> > //
> > typedef enum {
> > SdMmcNoData,
> > SdMmcPioMode,
> > SdMmcSdmaMode,
> > - SdMmcAdmaMode
> > + SdMmcAdma32bMode,
> > + SdMmcAdma64bV3Mode,
> > + SdMmcAdma64bV4Mode
> > } SD_MMC_HC_TRANSFER_MODE;
> >
> > //
> > +// The ADMA transfer lengths supported by SD Host Controller
> > +//
> > +typedef enum {
> > + SdMmcAdmaLen16b,
> > + SdMmcAdmaLen26b
> > +} SD_MMC_HC_ADMA_LEGTH;
>
> Typo 'LEGTH' -> 'LENGTH'
> Also, how about 'SD_MMC_HC_ADMA_LENGTH_MODE'?
>
> Best Regards,
> Hao Wu
>
> > +
> > +//
> > // The maximum data length of each descriptor line
> > //
> > #define ADMA_MAX_DATA_PER_LINE_16B SIZE_64KB
> > @@ -122,8 +131,20 @@ typedef struct {
> > UINT32 LowerLength:16;
> > UINT32 LowerAddress;
> > UINT32 UpperAddress;
> > +} SD_MMC_HC_ADMA_64_V3_DESC_LINE;
> > +
> > +typedef struct {
> > + UINT32 Valid:1;
> > + UINT32 End:1;
> > + UINT32 Int:1;
> > + UINT32 Reserved:1;
> > + UINT32 Act:2;
> > + UINT32 UpperLength:10;
> > + UINT32 LowerLength:16;
> > + UINT32 LowerAddress;
> > + UINT32 UpperAddress;
> > UINT32 Reserved1;
> > -} SD_MMC_HC_ADMA_64_DESC_LINE;
> > +} SD_MMC_HC_ADMA_64_V4_DESC_LINE;
> >
> > #define SD_MMC_SDMA_BOUNDARY 512 * 1024
> > #define SD_MMC_SDMA_ROUND_UP(x, n) (((x) + n) & ~(n - 1))
> > --
> > 2.7.4
> ________________________________
> This email message is for the sole use of the intended recipient(s) and may
> contain confidential information. Any unauthorized review, use, disclosure
> or distribution is prohibited. If you are not the intended recipient, please
> contact the sender by reply email and destroy all copies of the original
> message.
> ________________________________
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2019-03-07 1:34 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-03-01 18:30 [PATCH] MdeModulePkg/SdMmcPciHcDxe: Add V3 64b DMA Support Ashish Singhal
2019-03-02 16:00 ` Gao, Liming
2019-03-04 4:08 ` Ashish Singhal
2019-03-06 3:01 ` Wu, Hao A
2019-03-06 5:17 ` Ashish Singhal
2019-03-06 23:05 ` Cohen, Eugene
2019-03-06 23:06 ` Ashish Singhal
2019-03-07 0:54 ` Cohen, Eugene
2019-03-07 1:34 ` Wu, Hao A
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox