[edk2] [PATCH v2 0/5] ArmPlatformPkg: map VRAM using memory semantics

Ard Biesheuvel posted 5 patches 7 years, 6 months ago
Failed in applying to current master (apply log)
ArmPlatformPkg/ArmVExpressPkg/Include/Platform/RTSM/ArmPlatform.h                       | 10 ++++++----
ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressLibRTSM/RTSMMem.c                      |  8 +++++++-
ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c            | 14 +++++---------
ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpressLib.inf       |  3 ++-
ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c      | 14 +++++---------
ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpressLib.inf |  3 ++-
6 files changed, 27 insertions(+), 25 deletions(-)
[edk2] [PATCH v2 0/5] ArmPlatformPkg: map VRAM using memory semantics
Posted by Ard Biesheuvel 7 years, 6 months ago
As reported by Jeremy, the recently introduced accelerated memcpy/memset
routines are causing problems when used on framebuffer memory. While
framebuffers are arguably right on the blurry line between MMIO (with
side effects that are sensitive to ordering) and memory, mapping VRAM
as device memory is unnecessary to begin with, and so we can improve
our situation by using memory semantics for the VRAM mappings. (Whether
we end up reverting to the unaccelerated memcpy/memset routines is a
separate matter)

So fix both the FVP case, which has a separate VRAM region, and TC2, which
allocates VRAM from normal memory and changes the mapping attributes.

Note to Ryan: this fixes FVP Base model for me, but not the Foundation model
(whose PL111 support I never saw working to begin with, so I am not sure what
is going on there)

Ard Biesheuvel (5):
  ArmPlatformPkg/FVP: map motherboard VRAM as uncached memory
  ArmPlatformPkg/HdLcdArmVExpressLib: fix incorrect FreePool () call
  ArmPlatformPkg/PL111LcdArmVExpressLib: fix incorrect FreePool () call
  ArmPlatformPkg/HdLcdArmVExpressLib: use write-combine mapping for VRAM
  ArmPlatformPkg/PL111LcdArmVExpressLib: use write-combine mapping for
    VRAM

 ArmPlatformPkg/ArmVExpressPkg/Include/Platform/RTSM/ArmPlatform.h                       | 10 ++++++----
 ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressLibRTSM/RTSMMem.c                      |  8 +++++++-
 ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c            | 14 +++++---------
 ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpressLib.inf       |  3 ++-
 ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c      | 14 +++++---------
 ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpressLib.inf |  3 ++-
 6 files changed, 27 insertions(+), 25 deletions(-)

-- 
2.9.3

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v2 0/5] ArmPlatformPkg: map VRAM using memory semantics
Posted by Jeremy Linton 7 years, 6 months ago
Hi,

On 04/06/2017 08:15 AM, Ard Biesheuvel wrote:
> As reported by Jeremy, the recently introduced accelerated memcpy/memset
> routines are causing problems when used on framebuffer memory. While
> framebuffers are arguably right on the blurry line between MMIO (with
> side effects that are sensitive to ordering) and memory, mapping VRAM
> as device memory is unnecessary to begin with, and so we can improve
> our situation by using memory semantics for the VRAM mappings. (Whether
> we end up reverting to the unaccelerated memcpy/memset routines is a
> separate matter)
>
> So fix both the FVP case, which has a separate VRAM region, and TC2, which
> allocates VRAM from normal memory and changes the mapping attributes.
>
> Note to Ryan: this fixes FVP Base model for me, but not the Foundation model
> (whose PL111 support I never saw working to begin with, so I am not sure what
> is going on there)

So, I applied a similar set of patches against the juno, and everything 
continues to work. The change also looks good.

So,

Reviewed-by: Jeremy Linton <jeremy.linton@arm.com>


But I'm still concerned in general, since the HDLCD frame buffer is odd 
in that its just system memory rather than being attached to an IO bus 
of some form. AKA its not actually a "VRAM" (cue discussion about GDDR 
not having multiple ports/etc)...


>
> Ard Biesheuvel (5):
>   ArmPlatformPkg/FVP: map motherboard VRAM as uncached memory
>   ArmPlatformPkg/HdLcdArmVExpressLib: fix incorrect FreePool () call
>   ArmPlatformPkg/PL111LcdArmVExpressLib: fix incorrect FreePool () call
>   ArmPlatformPkg/HdLcdArmVExpressLib: use write-combine mapping for VRAM
>   ArmPlatformPkg/PL111LcdArmVExpressLib: use write-combine mapping for
>     VRAM
>
>  ArmPlatformPkg/ArmVExpressPkg/Include/Platform/RTSM/ArmPlatform.h                       | 10 ++++++----
>  ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressLibRTSM/RTSMMem.c                      |  8 +++++++-
>  ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c            | 14 +++++---------
>  ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpressLib.inf       |  3 ++-
>  ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c      | 14 +++++---------
>  ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpressLib.inf |  3 ++-
>  6 files changed, 27 insertions(+), 25 deletions(-)
>

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v2 0/5] ArmPlatformPkg: map VRAM using memory semantics
Posted by Ard Biesheuvel 7 years, 6 months ago
On 6 April 2017 at 17:29, Jeremy Linton <jeremy.linton@arm.com> wrote:
> Hi,
>
> On 04/06/2017 08:15 AM, Ard Biesheuvel wrote:
>>
>> As reported by Jeremy, the recently introduced accelerated memcpy/memset
>> routines are causing problems when used on framebuffer memory. While
>> framebuffers are arguably right on the blurry line between MMIO (with
>> side effects that are sensitive to ordering) and memory, mapping VRAM
>> as device memory is unnecessary to begin with, and so we can improve
>> our situation by using memory semantics for the VRAM mappings. (Whether
>> we end up reverting to the unaccelerated memcpy/memset routines is a
>> separate matter)
>>
>> So fix both the FVP case, which has a separate VRAM region, and TC2, which
>> allocates VRAM from normal memory and changes the mapping attributes.
>>
>> Note to Ryan: this fixes FVP Base model for me, but not the Foundation
>> model
>> (whose PL111 support I never saw working to begin with, so I am not sure
>> what
>> is going on there)
>
>
> So, I applied a similar set of patches against the juno, and everything
> continues to work. The change also looks good.
>
> So,
>
> Reviewed-by: Jeremy Linton <jeremy.linton@arm.com>
>

Thanks!

>
> But I'm still concerned in general, since the HDLCD frame buffer is odd in
> that its just system memory rather than being attached to an IO bus of some
> form. AKA its not actually a "VRAM" (cue discussion about GDDR not having
> multiple ports/etc)...
>
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v2 0/5] ArmPlatformPkg: map VRAM using memory semantics
Posted by Ryan Harkin 7 years, 6 months ago
On 6 April 2017 at 14:15, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> As reported by Jeremy, the recently introduced accelerated memcpy/memset
> routines are causing problems when used on framebuffer memory. While
> framebuffers are arguably right on the blurry line between MMIO (with
> side effects that are sensitive to ordering) and memory, mapping VRAM
> as device memory is unnecessary to begin with, and so we can improve
> our situation by using memory semantics for the VRAM mappings. (Whether
> we end up reverting to the unaccelerated memcpy/memset routines is a
> separate matter)
>
> So fix both the FVP case, which has a separate VRAM region, and TC2, which
> allocates VRAM from normal memory and changes the mapping attributes.
>
> Note to Ryan: this fixes FVP Base model for me, but not the Foundation model
> (whose PL111 support I never saw working to begin with, so I am not sure what
> is going on there)
>

With the fix to remove the duplicate VRAM_MOTHERBOARD_BASE, this
series is tested on Juno R0/1/2 to make sure they didn't break for
inexplicable reasons, on TC2 and FVP Base AEMv8 models.

FVP Foundation has never been verified with CLCD and it doesn't work
with or without this change, so I'm happy with that.

Tested-by: Ryan Harkin <ryan.harkin@linaro.org>


> Ard Biesheuvel (5):
>   ArmPlatformPkg/FVP: map motherboard VRAM as uncached memory
>   ArmPlatformPkg/HdLcdArmVExpressLib: fix incorrect FreePool () call
>   ArmPlatformPkg/PL111LcdArmVExpressLib: fix incorrect FreePool () call
>   ArmPlatformPkg/HdLcdArmVExpressLib: use write-combine mapping for VRAM
>   ArmPlatformPkg/PL111LcdArmVExpressLib: use write-combine mapping for
>     VRAM
>
>  ArmPlatformPkg/ArmVExpressPkg/Include/Platform/RTSM/ArmPlatform.h                       | 10 ++++++----
>  ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressLibRTSM/RTSMMem.c                      |  8 +++++++-
>  ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c            | 14 +++++---------
>  ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpressLib.inf       |  3 ++-
>  ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c      | 14 +++++---------
>  ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpressLib.inf |  3 ++-
>  6 files changed, 27 insertions(+), 25 deletions(-)
>
> --
> 2.9.3
>
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v2 0/5] ArmPlatformPkg: map VRAM using memory semantics
Posted by Leif Lindholm 7 years, 6 months ago
On Thu, Apr 06, 2017 at 02:15:46PM +0100, Ard Biesheuvel wrote:
> As reported by Jeremy, the recently introduced accelerated memcpy/memset
> routines are causing problems when used on framebuffer memory. While
> framebuffers are arguably right on the blurry line between MMIO (with
> side effects that are sensitive to ordering) and memory, mapping VRAM
> as device memory is unnecessary to begin with, and so we can improve
> our situation by using memory semantics for the VRAM mappings. (Whether
> we end up reverting to the unaccelerated memcpy/memset routines is a
> separate matter)
> 
> So fix both the FVP case, which has a separate VRAM region, and TC2, which
> allocates VRAM from normal memory and changes the mapping attributes.

For 2-5/5:
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

> 
> Note to Ryan: this fixes FVP Base model for me, but not the Foundation model
> (whose PL111 support I never saw working to begin with, so I am not sure what
> is going on there)
> 
> Ard Biesheuvel (5):
>   ArmPlatformPkg/FVP: map motherboard VRAM as uncached memory
>   ArmPlatformPkg/HdLcdArmVExpressLib: fix incorrect FreePool () call
>   ArmPlatformPkg/PL111LcdArmVExpressLib: fix incorrect FreePool () call
>   ArmPlatformPkg/HdLcdArmVExpressLib: use write-combine mapping for VRAM
>   ArmPlatformPkg/PL111LcdArmVExpressLib: use write-combine mapping for
>     VRAM
> 
>  ArmPlatformPkg/ArmVExpressPkg/Include/Platform/RTSM/ArmPlatform.h                       | 10 ++++++----
>  ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressLibRTSM/RTSMMem.c                      |  8 +++++++-
>  ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c            | 14 +++++---------
>  ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpressLib.inf       |  3 ++-
>  ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c      | 14 +++++---------
>  ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpressLib.inf |  3 ++-
>  6 files changed, 27 insertions(+), 25 deletions(-)
> 
> -- 
> 2.9.3
> 
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v2 0/5] ArmPlatformPkg: map VRAM using memory semantics
Posted by Ard Biesheuvel 7 years, 6 months ago
On 6 April 2017 at 19:29, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> On Thu, Apr 06, 2017 at 02:15:46PM +0100, Ard Biesheuvel wrote:
>> As reported by Jeremy, the recently introduced accelerated memcpy/memset
>> routines are causing problems when used on framebuffer memory. While
>> framebuffers are arguably right on the blurry line between MMIO (with
>> side effects that are sensitive to ordering) and memory, mapping VRAM
>> as device memory is unnecessary to begin with, and so we can improve
>> our situation by using memory semantics for the VRAM mappings. (Whether
>> we end up reverting to the unaccelerated memcpy/memset routines is a
>> separate matter)
>>
>> So fix both the FVP case, which has a separate VRAM region, and TC2, which
>> allocates VRAM from normal memory and changes the mapping attributes.
>
> For 2-5/5:
> Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
>

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