* [PATCH 0/2] EMMC2 fix _DMA for newer SOC
@ 2021-05-11 22:41 Jeremy Linton
2021-05-11 22:41 ` [PATCH 1/2] Platform/Raspberrypi: Update DMA constants based on SOC revision Jeremy Linton
2021-05-11 22:41 ` [PATCH 2/2] Platform/RaspberryPi: Invert emmc PIO/DMA selection Jeremy Linton
0 siblings, 2 replies; 6+ messages in thread
From: Jeremy Linton @ 2021-05-11 22:41 UTC (permalink / raw)
To: devel; +Cc: ardb+tianocore, pete, samer.el-haj-mahmoud, awarkentin,
Jeremy Linton
The _DMA range needs to change depending on the SOC in use, this
was part of the problem with enabling DMA by default because there
wasn't a clear way to determine the SOC revision in use.
Now that we have an id register for that, we can pick the correct
_DMA at runtime. Lets also flip DMA on by default.
Jeremy Linton (2):
Platform/Raspberrypi: Update DMA constants based on SOC revision
Platform/RaspberryPi: Invert emmc PIO/DMA selection
Platform/RaspberryPi/AcpiTables/Emmc.asl | 39 +++++++++++++++++++++-
.../RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr | 4 +--
Platform/RaspberryPi/RPi4/RPi4.dsc | 2 +-
.../Bcm27xx/Include/IndustryStandard/Bcm2711.h | 2 ++
4 files changed, 43 insertions(+), 4 deletions(-)
--
2.13.7
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] Platform/Raspberrypi: Update DMA constants based on SOC revision
2021-05-11 22:41 [PATCH 0/2] EMMC2 fix _DMA for newer SOC Jeremy Linton
@ 2021-05-11 22:41 ` Jeremy Linton
2021-05-12 11:19 ` Pete Batard
2021-05-11 22:41 ` [PATCH 2/2] Platform/RaspberryPi: Invert emmc PIO/DMA selection Jeremy Linton
1 sibling, 1 reply; 6+ messages in thread
From: Jeremy Linton @ 2021-05-11 22:41 UTC (permalink / raw)
To: devel; +Cc: ardb+tianocore, pete, samer.el-haj-mahmoud, awarkentin,
Jeremy Linton
The newer BCM2711 SoC's don't have a DMA constraint on the emmc2
controller. So we don't need to do the 1G translation. Lets
allow the AML to detect the SoC revision and return a different
_DMA resource.
Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
---
Platform/RaspberryPi/AcpiTables/Emmc.asl | 39 +++++++++++++++++++++-
.../Bcm27xx/Include/IndustryStandard/Bcm2711.h | 2 ++
2 files changed, 40 insertions(+), 1 deletion(-)
diff --git a/Platform/RaspberryPi/AcpiTables/Emmc.asl b/Platform/RaspberryPi/AcpiTables/Emmc.asl
index 23febe37b4..c6691e81dc 100644
--- a/Platform/RaspberryPi/AcpiTables/Emmc.asl
+++ b/Platform/RaspberryPi/AcpiTables/Emmc.asl
@@ -8,6 +8,7 @@
#include <IndustryStandard/Bcm2836SdHost.h>
#include <IndustryStandard/Bcm2836Sdio.h>
+#include <IndustryStandard/Bcm2711.h>
#include "AcpiTables.h"
@@ -31,7 +32,8 @@ DefinitionBlock (__FILE__, "SSDT", 2, "RPIFDN", "RPI4EMMC", 2)
Return (^RBUF)
}
- Name (_DMA, ResourceTemplate() {
+ // Translated DMA region for < C0
+ Name (DMTR, ResourceTemplate() {
QWordMemory (ResourceProducer,
,
MinFixed,
@@ -48,6 +50,41 @@ DefinitionBlock (__FILE__, "SSDT", 2, "RPIFDN", "RPI4EMMC", 2)
)
})
+ // Non translated DMA region for >= C0
+ Name (DMNT, ResourceTemplate() {
+ QWordMemory (ResourceProducer,
+ ,
+ MinFixed,
+ MaxFixed,
+ NonCacheable,
+ ReadWrite,
+ 0x0,
+ 0x0000000000000000, // MIN
+ 0x000000FFFFFFFFFF, // MAX
+ 0x0000000000000000, // TRA
+ 0x0000010000000000, // LEN
+ ,
+ ,
+ )
+ })
+
+ Method (_DMA, 0x0, Serialized)
+ {
+ OperationRegion (CHPR, SystemMemory, ID_CHIPREV, 0x4)
+ Field (CHPR, DWordAcc, NoLock, Preserve) {
+ SOCI, 32
+ }
+
+ if ((SOCI & 0xFF) >= 0x20)
+ {
+ return (^DMNT);
+ }
+ else
+ {
+ return (^DMTR);
+ }
+ }
+
// emmc2 Host Controller. (brcm,bcm2711-emmc2)
Device (SDC3)
{
diff --git a/Silicon/Broadcom/Bcm27xx/Include/IndustryStandard/Bcm2711.h b/Silicon/Broadcom/Bcm27xx/Include/IndustryStandard/Bcm2711.h
index 86906b2438..8a69128d11 100644
--- a/Silicon/Broadcom/Bcm27xx/Include/IndustryStandard/Bcm2711.h
+++ b/Silicon/Broadcom/Bcm27xx/Include/IndustryStandard/Bcm2711.h
@@ -88,4 +88,6 @@
#define THERM_SENSOR 0xfd5d2200
+#define ID_CHIPREV 0xfc404000
+
#endif /* BCM2711_H__ */
--
2.13.7
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] Platform/RaspberryPi: Invert emmc PIO/DMA selection
2021-05-11 22:41 [PATCH 0/2] EMMC2 fix _DMA for newer SOC Jeremy Linton
2021-05-11 22:41 ` [PATCH 1/2] Platform/Raspberrypi: Update DMA constants based on SOC revision Jeremy Linton
@ 2021-05-11 22:41 ` Jeremy Linton
2021-05-12 11:19 ` Pete Batard
1 sibling, 1 reply; 6+ messages in thread
From: Jeremy Linton @ 2021-05-11 22:41 UTC (permalink / raw)
To: devel; +Cc: ardb+tianocore, pete, samer.el-haj-mahmoud, awarkentin,
Jeremy Linton
Now that we are doing SoC detection and adjusting the DMA
window it should be safe to turn DMA on by default.
Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
---
Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr | 4 ++--
Platform/RaspberryPi/RPi4/RPi4.dsc | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
index aa124e4e31..759db6212f 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
@@ -300,8 +300,8 @@ formset
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;
+ option text = STRING_TOKEN(STR_MMC_EMMC_PIO), value = 0, flags = 0;
+ option text = STRING_TOKEN(STR_MMC_EMMC_DMA), value = 1, flags = DEFAULT;
endoneof;
endif;
#endif
diff --git a/Platform/RaspberryPi/RPi4/RPi4.dsc b/Platform/RaspberryPi/RPi4/RPi4.dsc
index cf796acf6a..b3acc04e07 100644
--- a/Platform/RaspberryPi/RPi4/RPi4.dsc
+++ b/Platform/RaspberryPi/RPi4/RPi4.dsc
@@ -486,7 +486,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
+ gRaspberryPiTokenSpaceGuid.PcdMmcEnableDma|L"MmcEnableDma"|gConfigDxeFormSetGuid|0x0|1
#
# Debug-related.
--
2.13.7
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] Platform/Raspberrypi: Update DMA constants based on SOC revision
2021-05-11 22:41 ` [PATCH 1/2] Platform/Raspberrypi: Update DMA constants based on SOC revision Jeremy Linton
@ 2021-05-12 11:19 ` Pete Batard
2021-05-12 12:22 ` Ard Biesheuvel
0 siblings, 1 reply; 6+ messages in thread
From: Pete Batard @ 2021-05-12 11:19 UTC (permalink / raw)
To: Jeremy Linton, devel; +Cc: ardb+tianocore, samer.el-haj-mahmoud, awarkentin
Two minor notes below:
On 2021.05.11 23:41, Jeremy Linton wrote:
> The newer BCM2711 SoC's don't have a DMA constraint on the emmc2
> controller. So we don't need to do the 1G translation. Lets
> allow the AML to detect the SoC revision and return a different
> _DMA resource.
>
> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
> ---
> Platform/RaspberryPi/AcpiTables/Emmc.asl | 39 +++++++++++++++++++++-
> .../Bcm27xx/Include/IndustryStandard/Bcm2711.h | 2 ++
> 2 files changed, 40 insertions(+), 1 deletion(-)
>
> diff --git a/Platform/RaspberryPi/AcpiTables/Emmc.asl b/Platform/RaspberryPi/AcpiTables/Emmc.asl
> index 23febe37b4..c6691e81dc 100644
> --- a/Platform/RaspberryPi/AcpiTables/Emmc.asl
> +++ b/Platform/RaspberryPi/AcpiTables/Emmc.asl
> @@ -8,6 +8,7 @@
>
> #include <IndustryStandard/Bcm2836SdHost.h>
> #include <IndustryStandard/Bcm2836Sdio.h>
> +#include <IndustryStandard/Bcm2711.h>
>
> #include "AcpiTables.h"
>
> @@ -31,7 +32,8 @@ DefinitionBlock (__FILE__, "SSDT", 2, "RPIFDN", "RPI4EMMC", 2)
> Return (^RBUF)
> }
>
> - Name (_DMA, ResourceTemplate() {
> + // Translated DMA region for < C0
Even if the code makes it clear that we're testing the chip revision to
decide what region to return, I would prefer if we had "for pre C0
revisions of the SoC" instead of "for < C0", as I suspect people who
read this too quickly, and have no idea what C0, refers to may think
we're talking about a DMA address boundary or something.
> + Name (DMTR, ResourceTemplate() {
> QWordMemory (ResourceProducer,
> ,
> MinFixed,
> @@ -48,6 +50,41 @@ DefinitionBlock (__FILE__, "SSDT", 2, "RPIFDN", "RPI4EMMC", 2)
> )
> })
>
> + // Non translated DMA region for >= C0
Same as above: "for post C0 revisions of the SoC"
> + Name (DMNT, ResourceTemplate() {
> + QWordMemory (ResourceProducer,
> + ,
> + MinFixed,
> + MaxFixed,
> + NonCacheable,
> + ReadWrite,
> + 0x0,
> + 0x0000000000000000, // MIN
> + 0x000000FFFFFFFFFF, // MAX
> + 0x0000000000000000, // TRA
> + 0x0000010000000000, // LEN
> + ,
> + ,
> + )
> + })
> +
> + Method (_DMA, 0x0, Serialized)
> + {
> + OperationRegion (CHPR, SystemMemory, ID_CHIPREV, 0x4)
> + Field (CHPR, DWordAcc, NoLock, Preserve) {
> + SOCI, 32
> + }
> +
> + if ((SOCI & 0xFF) >= 0x20)
> + {
> + return (^DMNT);
> + }
> + else
> + {
> + return (^DMTR);
> + }
> + }
> +
> // emmc2 Host Controller. (brcm,bcm2711-emmc2)
> Device (SDC3)
> {
> diff --git a/Silicon/Broadcom/Bcm27xx/Include/IndustryStandard/Bcm2711.h b/Silicon/Broadcom/Bcm27xx/Include/IndustryStandard/Bcm2711.h
> index 86906b2438..8a69128d11 100644
> --- a/Silicon/Broadcom/Bcm27xx/Include/IndustryStandard/Bcm2711.h
> +++ b/Silicon/Broadcom/Bcm27xx/Include/IndustryStandard/Bcm2711.h
> @@ -88,4 +88,6 @@
>
> #define THERM_SENSOR 0xfd5d2200
>
> +#define ID_CHIPREV 0xfc404000
> +
> #endif /* BCM2711_H__ */
>
With the comment changes, if agreed, applicable during integration:
Reviewed-by: Pete Batard <pete@akeo.ie>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] Platform/RaspberryPi: Invert emmc PIO/DMA selection
2021-05-11 22:41 ` [PATCH 2/2] Platform/RaspberryPi: Invert emmc PIO/DMA selection Jeremy Linton
@ 2021-05-12 11:19 ` Pete Batard
0 siblings, 0 replies; 6+ messages in thread
From: Pete Batard @ 2021-05-12 11:19 UTC (permalink / raw)
To: Jeremy Linton, devel; +Cc: ardb+tianocore, samer.el-haj-mahmoud, awarkentin
On 2021.05.11 23:41, Jeremy Linton wrote:
> Now that we are doing SoC detection and adjusting the DMA
> window it should be safe to turn DMA on by default.
>
> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
> ---
> Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr | 4 ++--
> Platform/RaspberryPi/RPi4/RPi4.dsc | 2 +-
> 2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
> index aa124e4e31..759db6212f 100644
> --- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
> +++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
> @@ -300,8 +300,8 @@ formset
> 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;
> + option text = STRING_TOKEN(STR_MMC_EMMC_PIO), value = 0, flags = 0;
> + option text = STRING_TOKEN(STR_MMC_EMMC_DMA), value = 1, flags = DEFAULT;
> endoneof;
> endif;
> #endif
> diff --git a/Platform/RaspberryPi/RPi4/RPi4.dsc b/Platform/RaspberryPi/RPi4/RPi4.dsc
> index cf796acf6a..b3acc04e07 100644
> --- a/Platform/RaspberryPi/RPi4/RPi4.dsc
> +++ b/Platform/RaspberryPi/RPi4/RPi4.dsc
> @@ -486,7 +486,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
> + gRaspberryPiTokenSpaceGuid.PcdMmcEnableDma|L"MmcEnableDma"|gConfigDxeFormSetGuid|0x0|1
>
> #
> # Debug-related.
>
Reviewed-by: Pete Batard <pete@akeo.ie>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] Platform/Raspberrypi: Update DMA constants based on SOC revision
2021-05-12 11:19 ` Pete Batard
@ 2021-05-12 12:22 ` Ard Biesheuvel
0 siblings, 0 replies; 6+ messages in thread
From: Ard Biesheuvel @ 2021-05-12 12:22 UTC (permalink / raw)
To: Pete Batard
Cc: Jeremy Linton, edk2-devel-groups-io, Ard Biesheuvel,
Samer El-Haj-Mahmoud, Andrei Warkentin
On Wed, 12 May 2021 at 13:19, Pete Batard <pete@akeo.ie> wrote:
>
> Two minor notes below:
>
> On 2021.05.11 23:41, Jeremy Linton wrote:
> > The newer BCM2711 SoC's don't have a DMA constraint on the emmc2
> > controller. So we don't need to do the 1G translation. Lets
> > allow the AML to detect the SoC revision and return a different
> > _DMA resource.
> >
> > Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
> > ---
> > Platform/RaspberryPi/AcpiTables/Emmc.asl | 39 +++++++++++++++++++++-
> > .../Bcm27xx/Include/IndustryStandard/Bcm2711.h | 2 ++
> > 2 files changed, 40 insertions(+), 1 deletion(-)
> >
> > diff --git a/Platform/RaspberryPi/AcpiTables/Emmc.asl b/Platform/RaspberryPi/AcpiTables/Emmc.asl
> > index 23febe37b4..c6691e81dc 100644
> > --- a/Platform/RaspberryPi/AcpiTables/Emmc.asl
> > +++ b/Platform/RaspberryPi/AcpiTables/Emmc.asl
> > @@ -8,6 +8,7 @@
> >
> > #include <IndustryStandard/Bcm2836SdHost.h>
> > #include <IndustryStandard/Bcm2836Sdio.h>
> > +#include <IndustryStandard/Bcm2711.h>
> >
> > #include "AcpiTables.h"
> >
> > @@ -31,7 +32,8 @@ DefinitionBlock (__FILE__, "SSDT", 2, "RPIFDN", "RPI4EMMC", 2)
> > Return (^RBUF)
> > }
> >
> > - Name (_DMA, ResourceTemplate() {
> > + // Translated DMA region for < C0
>
> Even if the code makes it clear that we're testing the chip revision to
> decide what region to return, I would prefer if we had "for pre C0
> revisions of the SoC" instead of "for < C0", as I suspect people who
> read this too quickly, and have no idea what C0, refers to may think
> we're talking about a DMA address boundary or something.
>
> > + Name (DMTR, ResourceTemplate() {
> > QWordMemory (ResourceProducer,
> > ,
> > MinFixed,
> > @@ -48,6 +50,41 @@ DefinitionBlock (__FILE__, "SSDT", 2, "RPIFDN", "RPI4EMMC", 2)
> > )
> > })
> >
> > + // Non translated DMA region for >= C0
>
> Same as above: "for post C0 revisions of the SoC"
>
> > + Name (DMNT, ResourceTemplate() {
> > + QWordMemory (ResourceProducer,
> > + ,
> > + MinFixed,
> > + MaxFixed,
> > + NonCacheable,
> > + ReadWrite,
> > + 0x0,
> > + 0x0000000000000000, // MIN
> > + 0x000000FFFFFFFFFF, // MAX
> > + 0x0000000000000000, // TRA
> > + 0x0000010000000000, // LEN
> > + ,
> > + ,
> > + )
> > + })
> > +
> > + Method (_DMA, 0x0, Serialized)
> > + {
> > + OperationRegion (CHPR, SystemMemory, ID_CHIPREV, 0x4)
> > + Field (CHPR, DWordAcc, NoLock, Preserve) {
> > + SOCI, 32
> > + }
> > +
> > + if ((SOCI & 0xFF) >= 0x20)
> > + {
> > + return (^DMNT);
> > + }
> > + else
> > + {
> > + return (^DMTR);
> > + }
> > + }
> > +
> > // emmc2 Host Controller. (brcm,bcm2711-emmc2)
> > Device (SDC3)
> > {
> > diff --git a/Silicon/Broadcom/Bcm27xx/Include/IndustryStandard/Bcm2711.h b/Silicon/Broadcom/Bcm27xx/Include/IndustryStandard/Bcm2711.h
> > index 86906b2438..8a69128d11 100644
> > --- a/Silicon/Broadcom/Bcm27xx/Include/IndustryStandard/Bcm2711.h
> > +++ b/Silicon/Broadcom/Bcm27xx/Include/IndustryStandard/Bcm2711.h
> > @@ -88,4 +88,6 @@
> >
> > #define THERM_SENSOR 0xfd5d2200
> >
> > +#define ID_CHIPREV 0xfc404000
> > +
> > #endif /* BCM2711_H__ */
> >
>
> With the comment changes, if agreed, applicable during integration:
> Reviewed-by: Pete Batard <pete@akeo.ie>
Thanks Pete. I have pushed these 2 patches as
e1efa61c33f7..09c5ab125b61 with the above suggestion applied.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-05-12 12:22 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-05-11 22:41 [PATCH 0/2] EMMC2 fix _DMA for newer SOC Jeremy Linton
2021-05-11 22:41 ` [PATCH 1/2] Platform/Raspberrypi: Update DMA constants based on SOC revision Jeremy Linton
2021-05-12 11:19 ` Pete Batard
2021-05-12 12:22 ` Ard Biesheuvel
2021-05-11 22:41 ` [PATCH 2/2] Platform/RaspberryPi: Invert emmc PIO/DMA selection Jeremy Linton
2021-05-12 11:19 ` Pete Batard
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox