[edk2,2/2] ArmPlatformPkg/PL111LcdArmVExpressLib: use write-combine mapping for VRAM

Message ID 1491424713-5203-2-git-send-email-ard.biesheuvel@linaro.org
State New
Headers show
Series
  • [edk2,1/2] ArmPlatformPkg/HdLcdArmVExpressLib: use write-combine mapping for VRAM
Related show

Commit Message

Ard Biesheuvel April 5, 2017, 8:38 p.m.
Replace the uncached memory mapping of the framebuffer with a write-
combining one. This improves performance, and avoids issues with
unaligned accesses and DC ZVA instructions performed by the accelerated
memcpy/memset routines.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

---
 ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
2.7.4

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Comments

Ryan Harkin April 6, 2017, 11:14 a.m. | #1
On 5 April 2017 at 21:38, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> Replace the uncached memory mapping of the framebuffer with a write-

> combining one. This improves performance, and avoids issues with

> unaligned accesses and DC ZVA instructions performed by the accelerated

> memcpy/memset routines.

>

> Contributed-under: TianoCore Contribution Agreement 1.0

> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>


Well, ... PL111 isn't usually enabled for me. And if I enable it,
neither Foundation nor AEMv8 models boot with or without this patch.

So it's no worse than before....


> ---

>  ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c | 2 +-

>  1 file changed, 1 insertion(+), 1 deletion(-)

>

> diff --git a/ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c b/ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c

> index 2000c9bdf436..d18d6b3e1665 100644

> --- a/ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c

> +++ b/ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c

> @@ -192,7 +192,7 @@ LcdPlatformGetVram (

>      ASSERT_EFI_ERROR(Status);

>

>      // Mark the VRAM as un-cachable. The VRAM is inside the DRAM, which is cachable.

> -    Status = Cpu->SetMemoryAttributes(Cpu, *VramBaseAddress, *VramSize, EFI_MEMORY_UC);

> +    Status = Cpu->SetMemoryAttributes(Cpu, *VramBaseAddress, *VramSize, EFI_MEMORY_WC);

>      ASSERT_EFI_ERROR(Status);

>      if (EFI_ERROR(Status)) {

>        gBS->FreePool(VramBaseAddress);

> --

> 2.7.4

>

> _______________________________________________

> edk2-devel mailing list

> edk2-devel@lists.01.org

> https://lists.01.org/mailman/listinfo/edk2-devel

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Ard Biesheuvel April 6, 2017, 11:32 a.m. | #2
On 6 April 2017 at 12:14, Ryan Harkin <ryan.harkin@linaro.org> wrote:
> On 5 April 2017 at 21:38, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:

>> Replace the uncached memory mapping of the framebuffer with a write-

>> combining one. This improves performance, and avoids issues with

>> unaligned accesses and DC ZVA instructions performed by the accelerated

>> memcpy/memset routines.

>>

>> Contributed-under: TianoCore Contribution Agreement 1.0

>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

>

> Well, ... PL111 isn't usually enabled for me. And if I enable it,

> neither Foundation nor AEMv8 models boot with or without this patch.

>

> So it's no worse than before....

>


Not even foundation model? That is strange ...

>

>> ---

>>  ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c | 2 +-

>>  1 file changed, 1 insertion(+), 1 deletion(-)

>>

>> diff --git a/ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c b/ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c

>> index 2000c9bdf436..d18d6b3e1665 100644

>> --- a/ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c

>> +++ b/ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c

>> @@ -192,7 +192,7 @@ LcdPlatformGetVram (

>>      ASSERT_EFI_ERROR(Status);

>>

>>      // Mark the VRAM as un-cachable. The VRAM is inside the DRAM, which is cachable.

>> -    Status = Cpu->SetMemoryAttributes(Cpu, *VramBaseAddress, *VramSize, EFI_MEMORY_UC);

>> +    Status = Cpu->SetMemoryAttributes(Cpu, *VramBaseAddress, *VramSize, EFI_MEMORY_WC);

>>      ASSERT_EFI_ERROR(Status);

>>      if (EFI_ERROR(Status)) {

>>        gBS->FreePool(VramBaseAddress);

>> --

>> 2.7.4

>>

>> _______________________________________________

>> edk2-devel mailing list

>> edk2-devel@lists.01.org

>> https://lists.01.org/mailman/listinfo/edk2-devel

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Ryan Harkin April 6, 2017, 11:43 a.m. | #3
On 6 April 2017 at 12:32, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> On 6 April 2017 at 12:14, Ryan Harkin <ryan.harkin@linaro.org> wrote:

>> On 5 April 2017 at 21:38, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:

>>> Replace the uncached memory mapping of the framebuffer with a write-

>>> combining one. This improves performance, and avoids issues with

>>> unaligned accesses and DC ZVA instructions performed by the accelerated

>>> memcpy/memset routines.

>>>

>>> Contributed-under: TianoCore Contribution Agreement 1.0

>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

>>

>> Well, ... PL111 isn't usually enabled for me. And if I enable it,

>> neither Foundation nor AEMv8 models boot with or without this patch.

>>

>> So it's no worse than before....

>>

>

> Not even foundation model? That is strange ...

>


The reason we created a config without PL111 was because Foundation
didn't originally have a PL111. And I wanted a single binary to run on
Foundation and AEMv8. But ARM added it to Foundation about a year ago
and I've never tried it.

I'm happy for you to submit the patch if it works for you.


>>

>>> ---

>>>  ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c | 2 +-

>>>  1 file changed, 1 insertion(+), 1 deletion(-)

>>>

>>> diff --git a/ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c b/ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c

>>> index 2000c9bdf436..d18d6b3e1665 100644

>>> --- a/ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c

>>> +++ b/ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c

>>> @@ -192,7 +192,7 @@ LcdPlatformGetVram (

>>>      ASSERT_EFI_ERROR(Status);

>>>

>>>      // Mark the VRAM as un-cachable. The VRAM is inside the DRAM, which is cachable.

>>> -    Status = Cpu->SetMemoryAttributes(Cpu, *VramBaseAddress, *VramSize, EFI_MEMORY_UC);

>>> +    Status = Cpu->SetMemoryAttributes(Cpu, *VramBaseAddress, *VramSize, EFI_MEMORY_WC);

>>>      ASSERT_EFI_ERROR(Status);

>>>      if (EFI_ERROR(Status)) {

>>>        gBS->FreePool(VramBaseAddress);

>>> --

>>> 2.7.4

>>>

>>> _______________________________________________

>>> edk2-devel mailing list

>>> edk2-devel@lists.01.org

>>> https://lists.01.org/mailman/listinfo/edk2-devel

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Patch hide | download patch | download mbox

diff --git a/ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c b/ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c
index 2000c9bdf436..d18d6b3e1665 100644
--- a/ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c
+++ b/ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c
@@ -192,7 +192,7 @@  LcdPlatformGetVram (
     ASSERT_EFI_ERROR(Status);
 
     // Mark the VRAM as un-cachable. The VRAM is inside the DRAM, which is cachable.
-    Status = Cpu->SetMemoryAttributes(Cpu, *VramBaseAddress, *VramSize, EFI_MEMORY_UC);
+    Status = Cpu->SetMemoryAttributes(Cpu, *VramBaseAddress, *VramSize, EFI_MEMORY_WC);
     ASSERT_EFI_ERROR(Status);
     if (EFI_ERROR(Status)) {
       gBS->FreePool(VramBaseAddress);