Re: [PATCH] Platform/Armada/Pp2Dxe: switch to ArmDmaLib

[Marcin Wojtas <mw@semihalf.com>]

2017-08-29 17:46 GMT+02:00 Leif Lindholm <leif.lindholm@linaro.org>:
> On Fri, Aug 25, 2017 at 01:25:54PM +0100, Ard Biesheuvel wrote:
>> Instead of hardcoding the non-cache coherent nature of this device
>> by invoking UncachedMemoryAllocationLib directly for allocating
>> shared buffers, switch to DmaLib, which encapsulates this at a
>> more abstract level. This allows the driver to be shared with
>> platforms that are cache coherent (by simply switching to another
>> DmaLib implementation), and removes the hardcoded dependency on
>> UncachedMemoryAllocationLib, which will be removed from upstream
>> EDK2.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>
> Ah, and there is the non-bogus fix :)
> Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

Please let me just run this on the HW before merge - I'll do it tomorrow.

Best regards,
Marcin

>
>> ---
>> This depends on patches #1 and #2 of
>>  https://lists.01.org/pipermail/edk2-devel/2017-August/013740.html
>>
>>  Platform/Marvell/Armada/Armada.dsc.inc         |  1 -
>>  Platform/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c   | 35 ++++++++++++++------
>>  Platform/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.h   |  2 +-
>>  Platform/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.inf |  4 +--
>>  4 files changed, 28 insertions(+), 14 deletions(-)
>>
>> diff --git a/Platform/Marvell/Armada/Armada.dsc.inc b/Platform/Marvell/Armada/Armada.dsc.inc
>> index 4e8f289fcbca..04b108619ac7 100644
>> --- a/Platform/Marvell/Armada/Armada.dsc.inc
>> +++ b/Platform/Marvell/Armada/Armada.dsc.inc
>> @@ -37,7 +37,6 @@
>>    UtmiPhyLib|Platform/Marvell/Library/UtmiPhyLib/UtmiPhyLib.inf
>>
>>    DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
>> -  UncachedMemoryAllocationLib|ArmPkg/Library/UncachedMemoryAllocationLib/UncachedMemoryAllocationLib.inf
>>    DebugPrintErrorLevelLib|MdePkg/Library/BaseDebugPrintErrorLevelLib/BaseDebugPrintErrorLevelLib.inf
>>
>>  # Basic utility libraries
>> diff --git a/Platform/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c b/Platform/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c
>> index 8de2473d09f0..1e2ccd0dbe9d 100644
>> --- a/Platform/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c
>> +++ b/Platform/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c
>> @@ -175,9 +175,10 @@ Pp2DxeBmPoolInit (
>>    VOID
>>    )
>>  {
>> -  INTN Index;
>> -  UINT8 *PoolAddr;
>> -  UINT32 PoolSize = (sizeof(VOID *) * MVPP2_BM_SIZE) * 2 + MVPP2_BM_POOL_PTR_ALIGN;
>> +  INTN          Index;
>> +  UINT8         *PoolAddr;
>> +  UINT32        PoolSize;
>> +  EFI_STATUS    Status;
>>
>>    ASSERT(MVPP2_BM_POOL_PTR_ALIGN >= sizeof(UINTN));
>>
>> @@ -194,11 +195,16 @@ Pp2DxeBmPoolInit (
>>      return EFI_OUT_OF_RESOURCES;
>>    }
>>
>> -  PoolAddr = UncachedAllocateAlignedZeroPool (PoolSize, MVPP2_BM_POOL_PTR_ALIGN);
>> -  if (PoolAddr == NULL) {
>> -    return EFI_OUT_OF_RESOURCES;
>> +  Status = DmaAllocateAlignedBuffer (EfiBootServicesData,
>> +                                     EFI_SIZE_TO_PAGES (PoolSize),
>> +                                     MVPP2_BM_POOL_PTR_ALIGN,
>> +                                     (VOID **)&PoolAddr);
>> +  if (EFI_ERROR (Status)) {
>> +    goto FreePools;
>>    }
>>
>> +  ZeroMem (PoolAddr, PoolSize);
>> +
>>    Mvpp2Shared->BmPools->Id = MVPP2_BM_POOL;
>>    Mvpp2Shared->BmPools->VirtAddr = (UINT32 *)PoolAddr;
>>    Mvpp2Shared->BmPools->PhysAddr = (UINTN)PoolAddr;
>> @@ -206,6 +212,10 @@ Pp2DxeBmPoolInit (
>>    Mvpp2BmPoolHwCreate(Mvpp2Shared, Mvpp2Shared->BmPools, MVPP2_BM_SIZE);
>>
>>    return EFI_SUCCESS;
>> +
>> +FreePools:
>> +  FreePool (Mvpp2Shared->BmPools);
>> +  return Status;
>>  }
>>
>>  /* Enable and fill BM pool */
>> @@ -1169,12 +1179,17 @@ Pp2DxeInitialise (
>>    Mvpp2Shared->Tclk = PcdGet32 (PcdPp2ClockFrequency);
>>
>>    /* Prepare buffers */
>> -  BufferSpace = UncachedAllocateAlignedZeroPool (BD_SPACE, MVPP2_BUFFER_ALIGN_SIZE);
>> -  if (BufferSpace == NULL) {
>> -    DEBUG((DEBUG_ERROR, "Failed to allocate buffer space\n"));
>> -    return EFI_OUT_OF_RESOURCES;
>> +  Status = DmaAllocateAlignedBuffer (EfiBootServicesData,
>> +                                     EFI_SIZE_TO_PAGES (BD_SPACE),
>> +                                     MVPP2_BUFFER_ALIGN_SIZE,
>> +                                     &BufferSpace);
>> +  if (EFI_ERROR (Status)) {
>> +    DEBUG ((DEBUG_ERROR, "Failed to allocate buffer space\n"));
>> +    return Status;
>>    }
>>
>> +  ZeroMem (BufferSpace, BD_SPACE);
>> +
>>    BufferLocation.TxDescs = BufferSpace;
>>    BufferLocation.AggrTxDescs = (MVPP2_TX_DESC *)((UINTN)BufferSpace + MVPP2_MAX_TXD * sizeof(MVPP2_TX_DESC));
>>    BufferLocation.RxDescs = (MVPP2_RX_DESC *)((UINTN)BufferSpace +
>> diff --git a/Platform/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.h b/Platform/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.h
>> index 3bb0c4a65376..a179638fd609 100644
>> --- a/Platform/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.h
>> +++ b/Platform/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.h
>> @@ -46,13 +46,13 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>>  #include <Library/BaseLib.h>
>>  #include <Library/BaseMemoryLib.h>
>>  #include <Library/DebugLib.h>
>> +#include <Library/DmaLib.h>
>>  #include <Library/IoLib.h>
>>  #include <Library/MemoryAllocationLib.h>
>>  #include <Library/NetLib.h>
>>  #include <Library/PcdLib.h>
>>  #include <Library/UefiBootServicesTableLib.h>
>>  #include <Library/UefiLib.h>
>> -#include <Library/UncachedMemoryAllocationLib.h>
>>
>>  #include "Mvpp2LibHw.h"
>>
>> diff --git a/Platform/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.inf b/Platform/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.inf
>> index 87cc5e8ded74..9052fe27b7f3 100644
>> --- a/Platform/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.inf
>> +++ b/Platform/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.inf
>> @@ -43,13 +43,14 @@
>>    Mvpp2Lib.c
>>
>>  [Packages]
>> +  EmbeddedPkg/EmbeddedPkg.dec
>>    MdePkg/MdePkg.dec
>>    MdeModulePkg/MdeModulePkg.dec
>> -  ArmPlatformPkg/ArmPlatformPkg.dec
>>    ArmPkg/ArmPkg.dec
>>    Platform/Marvell/Marvell.dec
>>
>>  [LibraryClasses]
>> +  DmaLib
>>    IoLib
>>    PcdLib
>>    BaseLib
>> @@ -60,7 +61,6 @@
>>    UefiDriverEntryPoint
>>    UefiBootServicesTableLib
>>    MemoryAllocationLib
>> -  UncachedMemoryAllocationLib
>>    CacheMaintenanceLib
>>
>>  [Protocols]
>> --
>> 2.11.0
>>