Hao, > I remember the commit b5547b9ce97e80c3127682a2a5d4b9bd14af353e from Ashish > only handles the controllers with version greater or equal to 4.00.
Right - that commit added support for SDHC 4.0 and above. The original driver supported SDHC 3.0 albeit only with SDMA and 32-bit ADMA support. With that commit two descriptor types are supported the 32-bit ADMA descriptor (SD_MMC_HC_ADMA_32_DESC_LINE which is 64-bits in size) and the V4 64-bit ADMA descriptor (SD_MMC_HC_ADMA_64_DESC_LINE which is 128-bits in size). However the commit mistakenly added a check for the V3 capability for 64-bit DMA and used it to set the PCI DUAL_ADDRESS_CYCLE attributre which then does not the 32-bit compatible bounce buffer mechanism. Later, when we attempt an ADMA data transfer we hit an ASSERT because the PCI DMA subsystem is not using bounce buffers to provide 32-bit DMA compatible memory. So the patch I submitted simply removes the unnecessary check of the V3 64-bit DMA capability check so the PCI DUAL_ADDRESS_CYCLE attribute is not set allowing 32-bit DMA to succeed on these platforms. > And the ADMA2 (96-bit Descriptor) mode for V3 controllers is selected by > setting the 'DMA Select' filed in the Host Control 1 Register to 11b. But > the currently behavior of the driver is setting the field to 10b, which I > think will not switch to the ADMA2 (96-bit Descriptor) mode for V3. Correct, right now for a V3 controller only 32-bit DMA is supported. An enhancement for V3 64-bit ADMA would improve performance on controllers that support that mode by eliminating the bounce buffer and associated memory copies. I think we should file a BZ for SD HCI V3 64-bit ADMA support - if you agree I would be happy to do that. I should point out that we have done extensive testing of this change on our host controller. Thanks, Eugene --- From: Wu, Hao A <hao.a...@intel.com> Sent: Wednesday, February 27, 2019 8:25 PM To: Cohen, Eugene <eug...@hp.com>; edk2-devel@lists.01.org Cc: Ashish Singhal <ashishsin...@nvidia.com> Subject: RE: [PATCH] MdeModulePkg/SdMmcPciHcDxe: Fix DMA on SDHC v3 64-bit systems Loop Ashish in. Some comments below. > -----Original Message----- > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of > Cohen, Eugene > Sent: Wednesday, February 27, 2019 6:59 PM > To: mailto:edk2-devel@lists.01.org; Wu, Hao A > Subject: [edk2] [PATCH] MdeModulePkg/SdMmcPciHcDxe: Fix DMA on SDHC > v3 64-bit systems > > The SdMmcPciHcDriverBindingStart function was checking two > different capability bits in determining whether 64-bit DMA > modes were supported, one mode is defined in the SDHC version > 3 specification (using 96-bit descriptors) and another is > defined in the SDHC version 4 specification (using 128-bit > descriptors). Since the currently implementation of 64-bit > ADMA2 only supports the SDHC version 4 implementation it is > incorrect to check the V3 64-bit capability bit since this > will activate V4 ADMA2 on V3 controllers. I remember the commit b5547b9ce97e80c3127682a2a5d4b9bd14af353e from Ashish only handles the controllers with version greater or equal to 4.00. And the ADMA2 (96-bit Descriptor) mode for V3 controllers is selected by setting the 'DMA Select' filed in the Host Control 1 Register to 11b. But the currently behavior of the driver is setting the field to 10b, which I think will not switch to the ADMA2 (96-bit Descriptor) mode for V3. Maybe there is something I miss here. Could you help to provide some more detail on the issue you met? Thanks. Best Regards, Hao Wu > > Cc: Hao Wu <mailto:hao.a...@intel.com> > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Eugene Cohen <mailto:eug...@hp.com> > --- > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c > b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c > index b474f8d..5bc91c5 100644 > --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c > +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c > @@ -666,8 +666,7 @@ SdMmcPciHcDriverBindingStart ( > // If any of the slots does not support 64b system bus > // do not enable 64b DMA in the PCI layer. > // > - if (Private->Capability[Slot].SysBus64V3 == 0 && > - Private->Capability[Slot].SysBus64V4 == 0) { > + if (Private->Capability[Slot].SysBus64V4 == 0) { > Support64BitDma = FALSE; > } > > -- > 2.7.4 > _______________________________________________ > edk2-devel mailing list > mailto: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
