Reviewed-by: Andrei Warkentin ________________________________ From: Pete Batard Sent: Monday, February 8, 2021 11:08 AM To: Jeremy Linton ; devel@edk2.groups.io Cc: Andrei Warkentin ; samer.el-haj-mahmoud@arm.com ; leif@nuviainc.com ; ardb+tianocore@kernel.org Subject: Re: [PATCH v2 3/4] Platform/RaspberryPi: User control of eMMC2 DMA On 2021.02.01 22:53, Jeremy Linton wrote: > DMA translation on the eMMC2 vary based on SOC, and > this is made worse by the poor _DMA support in linux. > > For now the "safe" option is to simply run the eMMC2 > controller in PIO mode. More advanced users or !linux > operating systems may choose to enable this to gain > a perf boost. > > Signed-off-by: Jeremy Linton > --- > Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c | 16 +++++++++++++++- > Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf | 1 + > Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni | 5 +++++ > Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr | 17 +++++++++++++++++ > Platform/RaspberryPi/Include/ConfigVars.h | 8 ++++++++ > Platform/RaspberryPi/RPi3/RPi3.dsc | 1 + > Platform/RaspberryPi/RPi4/RPi4.dsc | 1 + > Platform/RaspberryPi/RaspberryPi.dec | 1 + > 8 files changed, 49 insertions(+), 1 deletion(-) > > diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c > index 7f26f7b4be..1b8b360ddc 100644 > --- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c > +++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c > @@ -346,6 +346,15 @@ SetupVariables ( > } > > > > Size = sizeof (UINT32); > > + Status = gRT->GetVariable (L"MmcEnableDma", > > + &gConfigDxeFormSetGuid, > > + NULL, &Size, &Var32); > > + if (EFI_ERROR (Status)) { > > + Status = PcdSet32S (PcdMmcEnableDma, PcdGet32 (PcdMmcEnableDma)); > > + ASSERT_EFI_ERROR (Status); > > + } > > + > > + Size = sizeof (UINT32); > > Status = gRT->GetVariable (L"DebugEnableJTAG", > > &gConfigDxeFormSetGuid, > > NULL, &Size, &Var32); > > @@ -730,6 +739,11 @@ STATIC CONST AML_NAME_OP_REPLACE SsdtNameOpReplace[] = { > { } > > }; > > > > +STATIC CONST AML_NAME_OP_REPLACE SsdtEmmcNameOpReplace[] = { > > + { "SDMA", PcdToken (PcdMmcEnableDma) }, > > + { } > > +}; > > + > > STATIC CONST NAMESPACE_TABLES SdtTables[] = { > > { > > SIGNATURE_64 ('R', 'P', 'I', 'T', 'H', 'F', 'A', 'N'), > > @@ -741,7 +755,7 @@ STATIC CONST NAMESPACE_TABLES SdtTables[] = { > SIGNATURE_64 ('R', 'P', 'I', '4', 'E', 'M', 'M', 'C'), > > 0, > > PcdToken(PcdSdIsArasan), > > - NULL > > + SsdtEmmcNameOpReplace > > }, > > { > > SIGNATURE_64 ('R', 'P', 'I', 0, 0, 0, 0, 0), > > diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf > index 544e3b3e10..d51e54e010 100644 > --- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf > +++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf > @@ -84,6 +84,7 @@ > gRaspberryPiTokenSpaceGuid.PcdMmcSdDefaultSpeedMHz > > gRaspberryPiTokenSpaceGuid.PcdMmcSdHighSpeedMHz > > gRaspberryPiTokenSpaceGuid.PcdMmcDisableMulti > > + gRaspberryPiTokenSpaceGuid.PcdMmcEnableDma > > gRaspberryPiTokenSpaceGuid.PcdDebugEnableJTAG > > gRaspberryPiTokenSpaceGuid.PcdDisplayEnableScaledVModes > > gRaspberryPiTokenSpaceGuid.PcdDisplayEnableSShot > > diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni > index 2afe8f32ae..6abccc1fdb 100644 > --- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni > +++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni > @@ -94,6 +94,11 @@ > #string STR_MMC_SD_HS_PROMPT #language en-US "SD High Speed (MHz)" > > #string STR_MMC_SD_HS_HELP #language en-US "Override default 50Mhz" > > > > +#string STR_MMC_EMMC_PROMPT #language en-US "Enable eMMC DMA modes" > > +#string STR_MMC_EMMC_PIO #language en-US "PIO" > > +#string STR_MMC_EMMC_DMA #language en-US "SDMA/ADMA2" > > +#string STR_MMC_EMMC_HELP #language en-US "Enable eMMC DMA modes for OS's that support ACPI _DMA() translations" For consistency, since that's what we already use for other UI strings, the plural of OS should be "OSes" rather than "OS's" > > + (very minor) Extra line added here. > > > > /* > > * Display settings. > > diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr > index de5e43471a..cc7a09cfb7 100644 > --- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr > +++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr > @@ -96,6 +96,11 @@ formset > name = MmcSdHighSpeedMHz, > > guid = CONFIGDXE_FORM_SET_GUID; > > > > + efivarstore MMC_EMMC_DMA_VARSTORE_DATA, > > + attribute = EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_NON_VOLATILE, > > + name = MmcEnableDma, > > + guid = CONFIGDXE_FORM_SET_GUID; > > + > > efivarstore DEBUG_ENABLE_JTAG_VARSTORE_DATA, > > attribute = EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_NON_VOLATILE, > > name = DebugEnableJTAG, > > @@ -275,6 +280,18 @@ formset > maximum = 100, > > default = 50, > > endnumeric; > > +#if (RPI_MODEL == 4) > > + grayoutif ideqval SdIsArasan.Routing == 1; > > + oneof varid = MmcEnableDma.EnableDma, > > + prompt = STRING_TOKEN(STR_MMC_EMMC_PROMPT), > > + help = STRING_TOKEN(STR_MMC_EMMC_HELP), > > + flags = NUMERIC_SIZE_4 | INTERACTIVE | RESET_REQUIRED, > > + option text = STRING_TOKEN(STR_MMC_EMMC_PIO), value = 0, flags = DEFAULT; > > + option text = STRING_TOKEN(STR_MMC_EMMC_DMA), value = 1, flags = 0; > > + endoneof; > > + endif; > > +#endif > > + > > endform; > > > > form formid = 0x1004, > > diff --git a/Platform/RaspberryPi/Include/ConfigVars.h b/Platform/RaspberryPi/Include/ConfigVars.h > index c185bfe28b..142317985a 100644 > --- a/Platform/RaspberryPi/Include/ConfigVars.h > +++ b/Platform/RaspberryPi/Include/ConfigVars.h > @@ -135,4 +135,12 @@ typedef struct { > UINT32 MHz; > > } MMC_SD_HS_MHZ_VARSTORE_DATA; > > > > +typedef struct { > > + /* > > + * 0 - eMMC PIO mode > > + * 1 - eMMC DMA mode > > + */ > > + UINT32 EnableDma; > > +} MMC_EMMC_DMA_VARSTORE_DATA; > > + > > #endif /* CONFIG_VARS_H */ > > diff --git a/Platform/RaspberryPi/RPi3/RPi3.dsc b/Platform/RaspberryPi/RPi3/RPi3.dsc > index 530b42796a..107cbda297 100644 > --- a/Platform/RaspberryPi/RPi3/RPi3.dsc > +++ b/Platform/RaspberryPi/RPi3/RPi3.dsc > @@ -470,6 +470,7 @@ > gRaspberryPiTokenSpaceGuid.PcdMmcSdDefaultSpeedMHz|L"MmcSdDefaultSpeedMHz"|gConfigDxeFormSetGuid|0x0|25 > > gRaspberryPiTokenSpaceGuid.PcdMmcSdHighSpeedMHz|L"MmcSdHighSpeedMHz"|gConfigDxeFormSetGuid|0x0|50 > > gRaspberryPiTokenSpaceGuid.PcdMmcDisableMulti|L"MmcDisableMulti"|gConfigDxeFormSetGuid|0x0|0 > > + gRaspberryPiTokenSpaceGuid.PcdMmcEnableDma|L"MmcEnableDma"|gConfigDxeFormSetGuid|0x0|0 > > > > # > > # Debug-related. > > diff --git a/Platform/RaspberryPi/RPi4/RPi4.dsc b/Platform/RaspberryPi/RPi4/RPi4.dsc > index 5f8452aa0b..9962df0076 100644 > --- a/Platform/RaspberryPi/RPi4/RPi4.dsc > +++ b/Platform/RaspberryPi/RPi4/RPi4.dsc > @@ -481,6 +481,7 @@ > gRaspberryPiTokenSpaceGuid.PcdMmcSdDefaultSpeedMHz|L"MmcSdDefaultSpeedMHz"|gConfigDxeFormSetGuid|0x0|25 > > gRaspberryPiTokenSpaceGuid.PcdMmcSdHighSpeedMHz|L"MmcSdHighSpeedMHz"|gConfigDxeFormSetGuid|0x0|50 > > gRaspberryPiTokenSpaceGuid.PcdMmcDisableMulti|L"MmcDisableMulti"|gConfigDxeFormSetGuid|0x0|0 > > + gRaspberryPiTokenSpaceGuid.PcdMmcEnableDma|L"MmcEnableDma"|gConfigDxeFormSetGuid|0x0|0 > > > > # > > # Debug-related. > > diff --git a/Platform/RaspberryPi/RaspberryPi.dec b/Platform/RaspberryPi/RaspberryPi.dec > index 10723036aa..08135717ed 100644 > --- a/Platform/RaspberryPi/RaspberryPi.dec > +++ b/Platform/RaspberryPi/RaspberryPi.dec > @@ -69,3 +69,4 @@ > gRaspberryPiTokenSpaceGuid.PcdFanOnGpio|0|UINT32|0x0000001C > > gRaspberryPiTokenSpaceGuid.PcdFanTemp|0|UINT32|0x0000001D > > gRaspberryPiTokenSpaceGuid.PcdPlatformResetDelay|0|UINT32|0x0000001E > > + gRaspberryPiTokenSpaceGuid.PcdMmcEnableDma|0|UINT32|0x0000001F > Reviewed-by: Pete Batard