[Patch V4 0/3] Support SDMA mode on RPI4 target - 32bit

[Patch V4 0/3] Support SDMA mode on RPI4 target - 32bit

[Jaehoon Chung]

RPI4's SDHCI controller is supported SDMA mode. (Checked on kernel side)
But It doesn't use on u-boot side. Then it's too slow about read/write performance.
This patchset is supported SDMA mode on RPI4 target(32/64bit).
- Tested on RPI4 1GB/2GB/4GB target

Read/write time about 8MB file
Before
- Read : 1.472 seconds
- Write : 4.690 seconds
After
- Read : 0.359 seconds
- Write : 0.574 seconds

This patch is based on my RFC's patches.RPI4's SDHCI controller is supported SDMA mode. (Checked on kernel side)
But It doesn't use on u-boot side. Then it's too slow about read/write performance.
This patchset is supported SDMA mode on RPI4 target(32bit).
- I didn't test on RPI4 64bit.

Read/write time about 8MB file
Before
- Read : 1.472 seconds
- Write : 4.690 seconds
After
- Read : 0.359 seconds
- Write : 0.574 seconds

Changelog on V4
- Enable SDMA in rpi4_defconfig for 64bit

Changelog on V3
- Rebased on latest u-boot-mmc

Changelog on V2
- Keep printf message instead of debug
- Add Peng's Reviewed-by tag


Jaehoon Chung (3):
  mmc: sdhci: use phys2bus macro when dma address is accessed
  mmc: sdhci: not return error when SDMA is not supported
  configs: rpi_4 : enable SDHCI_SDMA config

 configs/rpi_4_32b_defconfig |  1 +
 configs/rpi_4_defconfig     |  1 +
 drivers/mmc/sdhci.c         | 13 +++++++------
 3 files changed, 9 insertions(+), 6 deletions(-)

-- 
2.26.0

[Patch V4 1/3] mmc: sdhci: use phys2bus macro when dma address is accessed

[Jaehoon Chung]

Use phys2bus macro when dma address is accessed.
Some targets need to use pyhs2bus macro. (e.g, RPI4)
After applied it, SDMA mode can be used.

Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
Reviewed-by: Peng Fan <peng.fan@nxp.com>
Reviewed-by: Minkyu Kang <mk7.kang@samsung.com>
---
 drivers/mmc/sdhci.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
index 520c9f9feb..2b7493fbac 100644
--- a/drivers/mmc/sdhci.c
+++ b/drivers/mmc/sdhci.c
@@ -16,6 +16,7 @@
 #include <sdhci.h>
 #include <dm.h>
 #include <linux/dma-mapping.h>
+#include <phys2bus.h>
 
 static void sdhci_reset(struct sdhci_host *host, u8 mask)
 {
@@ -150,7 +151,8 @@ static void sdhci_prepare_dma(struct sdhci_host *host, struct mmc_data *data,
 					  mmc_get_dma_dir(data));
 
 	if (host->flags & USE_SDMA) {
-		sdhci_writel(host, host->start_addr, SDHCI_DMA_ADDRESS);
+		sdhci_writel(host, phys_to_bus((ulong)host->start_addr),
+				SDHCI_DMA_ADDRESS);
 	} else if (host->flags & (USE_ADMA | USE_ADMA64)) {
 		sdhci_prepare_adma_table(host, data);
 
@@ -204,7 +206,7 @@ static int sdhci_transfer_data(struct sdhci_host *host, struct mmc_data *data)
 				start_addr &=
 				~(SDHCI_DEFAULT_BOUNDARY_SIZE - 1);
 				start_addr += SDHCI_DEFAULT_BOUNDARY_SIZE;
-				sdhci_writel(host, start_addr,
+				sdhci_writel(host, phys_to_bus((ulong)start_addr),
 					     SDHCI_DMA_ADDRESS);
 			}
 		}
-- 
2.26.0

[Patch V4 2/3] mmc: sdhci: not return error when SDMA is not supported

[Jaehoon Chung]

If Host controller doesn't support SDMA, it doesn't need to return
error. Because it can be worked with PIO mode.

Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
Reviewed-by: Peng Fan <peng.fan@nxp.com>
Reviewed-by: Minkyu Kang <mk7.kang@samsung.com>
---
 drivers/mmc/sdhci.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
index 2b7493fbac..49e67fc7bd 100644
--- a/drivers/mmc/sdhci.c
+++ b/drivers/mmc/sdhci.c
@@ -741,13 +741,12 @@ int sdhci_setup_cfg(struct mmc_config *cfg, struct sdhci_host *host,
 	debug("%s, caps: 0x%x\n", __func__, caps);
 
 #ifdef CONFIG_MMC_SDHCI_SDMA
-	if (!(caps & SDHCI_CAN_DO_SDMA)) {
+	if ((caps & SDHCI_CAN_DO_SDMA)) {
+		host->flags |= USE_SDMA;
+	} else {
 		printf("%s: Your controller doesn't support SDMA!!\n",
 		       __func__);
-		return -EINVAL;
 	}
-
-	host->flags |= USE_SDMA;
 #endif
 #if CONFIG_IS_ENABLED(MMC_SDHCI_ADMA)
 	if (!(caps & SDHCI_CAN_DO_ADMA2)) {
-- 
2.26.0

[Patch V4 3/3] configs: rpi_4 : enable SDHCI_SDMA config

[Jaehoon Chung]

Enable SDHCI_SDMA configuration.

Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
Reviewed-by: Peng Fan <peng.fan@nxp.com>
Reviewed-by: Minkyu Kang <mk7.kang@samsung.com>
---
 configs/rpi_4_32b_defconfig | 1 +
 configs/rpi_4_defconfig     | 1 +
 2 files changed, 2 insertions(+)

diff --git a/configs/rpi_4_32b_defconfig b/configs/rpi_4_32b_defconfig
index 72cda5d949..7189914606 100644
--- a/configs/rpi_4_32b_defconfig
+++ b/configs/rpi_4_32b_defconfig
@@ -25,6 +25,7 @@ CONFIG_DFU_MMC=y
 CONFIG_DM_KEYBOARD=y
 CONFIG_DM_MMC=y
 CONFIG_MMC_SDHCI=y
+CONFIG_MMC_SDHCI_SDMA=y
 CONFIG_MMC_SDHCI_BCM2835=y
 CONFIG_DM_ETH=y
 CONFIG_BCMGENET=y
diff --git a/configs/rpi_4_defconfig b/configs/rpi_4_defconfig
index 6d148dab07..454d28ea2b 100644
--- a/configs/rpi_4_defconfig
+++ b/configs/rpi_4_defconfig
@@ -25,6 +25,7 @@ CONFIG_DFU_MMC=y
 CONFIG_DM_KEYBOARD=y
 CONFIG_DM_MMC=y
 CONFIG_MMC_SDHCI=y
+CONFIG_MMC_SDHCI_SDMA=y
 CONFIG_MMC_SDHCI_BCM2835=y
 CONFIG_DM_ETH=y
 CONFIG_BCMGENET=y
-- 
2.26.0

[Patch V4 3/3] configs: rpi_4 : enable SDHCI_SDMA config

[Matthias Brugger]

Hi Jaehoon,

On 27/03/2020 05:08, Jaehoon Chung wrote:
> Enable SDHCI_SDMA configuration.
> 
> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
> Reviewed-by: Peng Fan <peng.fan@nxp.com>
> Reviewed-by: Minkyu Kang <mk7.kang@samsung.com>
> ---
>  configs/rpi_4_32b_defconfig | 1 +
>  configs/rpi_4_defconfig     | 1 +
>  2 files changed, 2 insertions(+)
> 

Please address the comments I had in v3 of this patch:
https://patchwork.ozlabs.org/patch/1261047/

If you just send a new version of the patch that won't convince me to take it.
We will need to make sure that we are fine with patch. Especially I'm concerned
about the limitation of the device to only be able to access the first GiB of
RAM for DMA. I'd like to see an explanation why this won't happen on U-Boot.

Regards,
Matthias

> diff --git a/configs/rpi_4_32b_defconfig b/configs/rpi_4_32b_defconfig
> index 72cda5d949..7189914606 100644
> --- a/configs/rpi_4_32b_defconfig
> +++ b/configs/rpi_4_32b_defconfig
> @@ -25,6 +25,7 @@ CONFIG_DFU_MMC=y
>  CONFIG_DM_KEYBOARD=y
>  CONFIG_DM_MMC=y
>  CONFIG_MMC_SDHCI=y
> +CONFIG_MMC_SDHCI_SDMA=y
>  CONFIG_MMC_SDHCI_BCM2835=y
>  CONFIG_DM_ETH=y
>  CONFIG_BCMGENET=y
> diff --git a/configs/rpi_4_defconfig b/configs/rpi_4_defconfig
> index 6d148dab07..454d28ea2b 100644
> --- a/configs/rpi_4_defconfig
> +++ b/configs/rpi_4_defconfig
> @@ -25,6 +25,7 @@ CONFIG_DFU_MMC=y
>  CONFIG_DM_KEYBOARD=y
>  CONFIG_DM_MMC=y
>  CONFIG_MMC_SDHCI=y
> +CONFIG_MMC_SDHCI_SDMA=y
>  CONFIG_MMC_SDHCI_BCM2835=y
>  CONFIG_DM_ETH=y
>  CONFIG_BCMGENET=y
> 

[Patch V4 3/3] configs: rpi_4 : enable SDHCI_SDMA config

[Jaehoon Chung]

Hi Matthias,

On 3/27/20 10:12 PM, Matthias Brugger wrote:
> Hi Jaehoon,
> 
> On 27/03/2020 05:08, Jaehoon Chung wrote:
>> Enable SDHCI_SDMA configuration.
>>
>> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
>> Reviewed-by: Peng Fan <peng.fan@nxp.com>
>> Reviewed-by: Minkyu Kang <mk7.kang@samsung.com>
>> ---
>>  configs/rpi_4_32b_defconfig | 1 +
>>  configs/rpi_4_defconfig     | 1 +
>>  2 files changed, 2 insertions(+)
>>
> 
> Please address the comments I had in v3 of this patch:
> https://protect2.fireeye.com/url?k=9b37681b-c6e760fe-9b36e354-000babff3563-8e3527bcf915de86&u=https://patchwork.ozlabs.org/patch/1261047/

Thanks for ping to me. I missed it.

> 
> If you just send a new version of the patch that won't convince me to take it.
> We will need to make sure that we are fine with patch. Especially I'm concerned
> about the limitation of the device to only be able to access the first GiB of
> RAM for DMA. I'd like to see an explanation why this won't happen on U-Boot.

It's why used phys_to_bus function.(arch/arm/mach-bcm283x/phys2bus.c)
It's returned the bus address to access ram with DMA. Based at 0xc0000000 or 0x40000000.


Best Regards,
Jaehoon Chung

> 
> Regards,
> Matthias
> 
>> diff --git a/configs/rpi_4_32b_defconfig b/configs/rpi_4_32b_defconfig
>> index 72cda5d949..7189914606 100644
>> --- a/configs/rpi_4_32b_defconfig
>> +++ b/configs/rpi_4_32b_defconfig
>> @@ -25,6 +25,7 @@ CONFIG_DFU_MMC=y
>>  CONFIG_DM_KEYBOARD=y
>>  CONFIG_DM_MMC=y
>>  CONFIG_MMC_SDHCI=y
>> +CONFIG_MMC_SDHCI_SDMA=y
>>  CONFIG_MMC_SDHCI_BCM2835=y
>>  CONFIG_DM_ETH=y
>>  CONFIG_BCMGENET=y
>> diff --git a/configs/rpi_4_defconfig b/configs/rpi_4_defconfig
>> index 6d148dab07..454d28ea2b 100644
>> --- a/configs/rpi_4_defconfig
>> +++ b/configs/rpi_4_defconfig
>> @@ -25,6 +25,7 @@ CONFIG_DFU_MMC=y
>>  CONFIG_DM_KEYBOARD=y
>>  CONFIG_DM_MMC=y
>>  CONFIG_MMC_SDHCI=y
>> +CONFIG_MMC_SDHCI_SDMA=y
>>  CONFIG_MMC_SDHCI_BCM2835=y
>>  CONFIG_DM_ETH=y
>>  CONFIG_BCMGENET=y
>>
> 
> 

[Patch V4 3/3] configs: rpi_4 : enable SDHCI_SDMA config

[Jaehoon Chung]

Hi,

On 3/30/20 1:19 PM, Jaehoon Chung wrote:
> Hi Matthias,
> 
> On 3/27/20 10:12 PM, Matthias Brugger wrote:
>> Hi Jaehoon,
>>
>> On 27/03/2020 05:08, Jaehoon Chung wrote:
>>> Enable SDHCI_SDMA configuration.
>>>
>>> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
>>> Reviewed-by: Peng Fan <peng.fan@nxp.com>
>>> Reviewed-by: Minkyu Kang <mk7.kang@samsung.com>
>>> ---
>>>  configs/rpi_4_32b_defconfig | 1 +
>>>  configs/rpi_4_defconfig     | 1 +
>>>  2 files changed, 2 insertions(+)
>>>
>>
>> Please address the comments I had in v3 of this patch:
>> https://protect2.fireeye.com/url?k=9b37681b-c6e760fe-9b36e354-000babff3563-8e3527bcf915de86&u=https://patchwork.ozlabs.org/patch/1261047/
> 
> Thanks for ping to me. I missed it.
> 
>>
>> If you just send a new version of the patch that won't convince me to take it.
>> We will need to make sure that we are fine with patch. Especially I'm concerned
>> about the limitation of the device to only be able to access the first GiB of
>> RAM for DMA. I'd like to see an explanation why this won't happen on U-Boot.
> 
> It's why used phys_to_bus function.(arch/arm/mach-bcm283x/phys2bus.c)
> It's returned the bus address to access ram with DMA. Based at 0xc0000000 or 0x40000000.

Is there any comment? 

Best Regards,
Jaehoon Chung

> 
> 
> Best Regards,
> Jaehoon Chung
> 
>>
>> Regards,
>> Matthias
>>
>>> diff --git a/configs/rpi_4_32b_defconfig b/configs/rpi_4_32b_defconfig
>>> index 72cda5d949..7189914606 100644
>>> --- a/configs/rpi_4_32b_defconfig
>>> +++ b/configs/rpi_4_32b_defconfig
>>> @@ -25,6 +25,7 @@ CONFIG_DFU_MMC=y
>>>  CONFIG_DM_KEYBOARD=y
>>>  CONFIG_DM_MMC=y
>>>  CONFIG_MMC_SDHCI=y
>>> +CONFIG_MMC_SDHCI_SDMA=y
>>>  CONFIG_MMC_SDHCI_BCM2835=y
>>>  CONFIG_DM_ETH=y
>>>  CONFIG_BCMGENET=y
>>> diff --git a/configs/rpi_4_defconfig b/configs/rpi_4_defconfig
>>> index 6d148dab07..454d28ea2b 100644
>>> --- a/configs/rpi_4_defconfig
>>> +++ b/configs/rpi_4_defconfig
>>> @@ -25,6 +25,7 @@ CONFIG_DFU_MMC=y
>>>  CONFIG_DM_KEYBOARD=y
>>>  CONFIG_DM_MMC=y
>>>  CONFIG_MMC_SDHCI=y
>>> +CONFIG_MMC_SDHCI_SDMA=y
>>>  CONFIG_MMC_SDHCI_BCM2835=y
>>>  CONFIG_DM_ETH=y
>>>  CONFIG_BCMGENET=y
>>>
>>
>>
> 
> 
> 

[Patch V4 3/3] configs: rpi_4 : enable SDHCI_SDMA config

[Marek Szyprowski]

Hi Matthias and Jaehoon,

On 27.03.2020 14:12, Matthias Brugger wrote:
> On 27/03/2020 05:08, Jaehoon Chung wrote:
>> Enable SDHCI_SDMA configuration.
>>
>> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
>> Reviewed-by: Peng Fan <peng.fan@nxp.com>
>> Reviewed-by: Minkyu Kang <mk7.kang@samsung.com>
>> ---
>>   configs/rpi_4_32b_defconfig | 1 +
>>   configs/rpi_4_defconfig     | 1 +
>>   2 files changed, 2 insertions(+)
>>
> Please address the comments I had in v3 of this patch:
> https://protect2.fireeye.com/url?k=006fd2e9-5dbfda25-006e59a6-000babff3793-1cb73b2d76c550f7&u=https://patchwork.ozlabs.org/patch/1261047/
>
> If you just send a new version of the patch that won't convince me to take it.
> We will need to make sure that we are fine with patch. Especially I'm concerned
> about the limitation of the device to only be able to access the first GiB of
> RAM for DMA. I'd like to see an explanation why this won't happen on U-Boot.

According to my investigation related to the PCIe XHCI driver and its DMA:

https://patchwork.ozlabs.org/project/uboot/patch/20200424165012.31915-10-s.nawrocki at samsung.com/#2418327

transfers to any buffer which has been allocated with malloc/memalign 
are safe. Malloc region is initialized in runtime and placed nearly by 
the end of the first memory bank, which in case of Pi4 is always below 
the first GiB. Transfers to the arbitrary regions above the first GiB 
will fail. Adding a check for such case might be a good idea. Later one 
can even add a fallback with temporary malloc buffer (or PIO mode if 
switching on fly is possible) and memcpy for the transfers above the 
first GiB if there is such need.

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

[Patch V4 0/3] Support SDMA mode on RPI4 target - 32bit

[Matthias Brugger]

On 27/03/2020 05:07, Jaehoon Chung wrote:
> RPI4's SDHCI controller is supported SDMA mode. (Checked on kernel side)
> But It doesn't use on u-boot side. Then it's too slow about read/write performance.
> This patchset is supported SDMA mode on RPI4 target(32/64bit).
> - Tested on RPI4 1GB/2GB/4GB target

Whole series queued for v2020.07

Thanks a lot!
Matthias

> 
> Read/write time about 8MB file
> Before
> - Read : 1.472 seconds
> - Write : 4.690 seconds
> After
> - Read : 0.359 seconds
> - Write : 0.574 seconds
> 
> This patch is based on my RFC's patches.RPI4's SDHCI controller is supported SDMA mode. (Checked on kernel side)
> But It doesn't use on u-boot side. Then it's too slow about read/write performance.
> This patchset is supported SDMA mode on RPI4 target(32bit).
> - I didn't test on RPI4 64bit.
> 
> Read/write time about 8MB file
> Before
> - Read : 1.472 seconds
> - Write : 4.690 seconds
> After
> - Read : 0.359 seconds
> - Write : 0.574 seconds
> 
> Changelog on V4
> - Enable SDMA in rpi4_defconfig for 64bit
> 
> Changelog on V3
> - Rebased on latest u-boot-mmc
> 
> Changelog on V2
> - Keep printf message instead of debug
> - Add Peng's Reviewed-by tag
> 
> 
> Jaehoon Chung (3):
>   mmc: sdhci: use phys2bus macro when dma address is accessed
>   mmc: sdhci: not return error when SDMA is not supported
>   configs: rpi_4 : enable SDHCI_SDMA config
> 
>  configs/rpi_4_32b_defconfig |  1 +
>  configs/rpi_4_defconfig     |  1 +
>  drivers/mmc/sdhci.c         | 13 +++++++------
>  3 files changed, 9 insertions(+), 6 deletions(-)
>