eMMC performance regression caused by 427b6514d0953bf

PATCH Superseded Jisheng Zhang 3 participants 11 messages Oct 13, 2020 – Oct 15, 2020

Subsystems (2)

MMC, SD and SDIO Secure digital host controller interface (SDHCI) driver

lore

Share & tools

lore

Jisheng Zhang

author

2020-10-13 11:09 UTC #0 lore raw

Hi,

I found an emmc performance regression with below simple benchmark cmd:

dd if=/dev/mmcblk0 of=/dev/null bs=8192 count=100000

This regression has been bisected to commit 427b6514d0953bf (“mmc: sdhci: Add Auto CMD Auto Select support”)

If I revert this commit, the performance is good now.

I’m not sure whether this is common issue or not.

Thanks

Ulf Hansson

2020-10-13 12:33 UTC #1 lore raw

+ Adrian

Quoted 5 lines · Jisheng Zhang

Hi,
I found an emmc performance regression with below simple benchmark cmd:

Quoted 8 lines

dd if=/dev/mmcblk0 of=/dev/null bs=8192 count=100000
This regression has been bisected to commit 427b6514d0953bf (“mmc: sdhci: Add Auto CMD Auto Select support”)
If I revert this commit, the performance is good now.
I’m not sure whether this is common issue or not.

Can you provide some data, like how much is the regression and how many tests did you run?

Did you try different block sizes of the dd command?

Kind regards
Uffe

Adrian Hunter

maintainer

2020-10-13 13:23 UTC #2 lore raw

Quoted 13 lines · Ulf Hansson

+ Adrian
Quoted 10 lines · Jisheng Zhang
Hi,
I found an emmc performance regression with below simple benchmark cmd:
dd if=/dev/mmcblk0 of=/dev/null bs=8192 count=100000
This regression has been bisected to commit 427b6514d0953bf (“mmc: sdhci: Add Auto CMD Auto Select support”)

Quoted 8 lines

Quoted 3 lines
If I revert this commit, the performance is good now.
I’m not sure whether this is common issue or not.
Can you provide some data, like how much is the regression and how many tests did you run?
Did you try different block sizes of the dd command?

Also what is the SDHCI version? The patch in question is for version >= 4.1

What driver?

Can you tell if the patch changed which auto command is used?

Jisheng Zhang

author

2020-10-14 10:33 UTC #3 lore raw

PATCH 1/1 mmc: sdhci: Use Auto CMD Auto Select only when v4_mode is true

Auto CMD Auto Select can only be used when v4_mode is enabled.

Fixes 427b6514d095 ("mmc: sdhci: Add Auto CMD Auto Select support")

Signed-off-by Jisheng Zhang <[email protected]>

---
 drivers/mmc/host/sdhci.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

drivers/mmc/host/sdhci.c +2 -1

index 592a55a34b58..5e0ec5df4074 100644
@@ -1386,7 +1386,8 @@ static inline void sdhci_auto_cmd_select(struct sdhci_host *host,
 	 * Select' is recommended rather than use of 'Auto CMD12
 	 * Enable' or 'Auto CMD23 Enable'.
 	 */
-	if (host->version >= SDHCI_SPEC_410 && (use_cmd12 || use_cmd23)) {
+	if (host->version >= SDHCI_SPEC_410 && host->v4_mode &&
+	    (use_cmd12 || use_cmd23)) {
 		*mode |= SDHCI_TRNS_AUTO_SEL;
 
 		ctrl2 = sdhci_readw(host, SDHCI_HOST_CONTROL2);

Signature · 1 line

2.28.0

Adrian Hunter

maintainer

2020-10-14 19:45 UTC #4 lore raw

Re: [PATCH] mmc: sdhci: Use Auto CMD Auto Select only when v4_mode is true

+ Chunyan

Quoted 1 line · Jisheng Zhang

Auto CMD Auto Select can only be used when v4_mode is enabled.

The SDHCI spec. doesn't seem to say that. AFAICS it refers only to v4.1 not v4 mode.

Quoted 22 lines

Fixes 427b6514d095 ("mmc: sdhci: Add Auto CMD Auto Select support")

Signed-off-by Jisheng Zhang <[email protected]>

---
 drivers/mmc/host/sdhci.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

drivers/mmc/host/sdhci.c +2 -1

index 592a55a34b58..5e0ec5df4074 100644
@@ -1386,7 +1386,8 @@ static inline void sdhci_auto_cmd_select(struct sdhci_host *host,
 	 * Select' is recommended rather than use of 'Auto CMD12
 	 * Enable' or 'Auto CMD23 Enable'.
 	 */
-	if (host->version >= SDHCI_SPEC_410 && (use_cmd12 || use_cmd23)) {
+	if (host->version >= SDHCI_SPEC_410 && host->v4_mode &&
+	    (use_cmd12 || use_cmd23)) {
 		*mode |= SDHCI_TRNS_AUTO_SEL;
 
 		ctrl2 = sdhci_readw(host, SDHCI_HOST_CONTROL2);

Jisheng Zhang

author

2020-10-15 02:12 UTC #5 lore raw

Re: [PATCH] mmc: sdhci: Use Auto CMD Auto Select only when v4_mode is true

Quoted 9 lines · On Wed, 14 Oct 2020 22:44:50 +0300 Adrian Hunter wrote:

+ Chunyan
Quoted 1 line · Jisheng Zhang
Auto CMD Auto Select can only be used when v4_mode is enabled.
The SDHCI spec. doesn't seem to say that. AFAICS it refers only to v4.1 not v4 mode.

4.10 defines the "Auto CMD Auto Select" mode, v4 mode bit in SDHCI_HOST_CONTROL2 selects V3 compatible or V4 compatible mode, I think the v4 here includes v4.0, v4.1 and v4.2, so if we want to use the mode we have to ensure v4_mode is enabled.

thanks

Quoted 24 lines

Quoted 23 lines

Fixes 427b6514d095 ("mmc: sdhci: Add Auto CMD Auto Select support")

Signed-off-by Jisheng Zhang <[email protected]>

---
 drivers/mmc/host/sdhci.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

drivers/mmc/host/sdhci.c +2 -1

index 592a55a34b58..5e0ec5df4074 100644
@@ -1386,7 +1386,8 @@ static inline void sdhci_auto_cmd_select(struct sdhci_host *host,
       * Select' is recommended rather than use of 'Auto CMD12
       * Enable' or 'Auto CMD23 Enable'.
       */
-     if (host->version >= SDHCI_SPEC_410 && (use_cmd12 || use_cmd23)) {
+     if (host->version >= SDHCI_SPEC_410 && host->v4_mode &&
+         (use_cmd12 || use_cmd23)) {
              *mode |= SDHCI_TRNS_AUTO_SEL;
 
              ctrl2 = sdhci_readw(host, SDHCI_HOST_CONTROL2);

Jisheng Zhang

author

2020-10-15 02:39 UTC #6 lore raw

Re: [PATCH] mmc: sdhci: Use Auto CMD Auto Select only when v4_mode is true

Quoted 8 lines · On Thu, 15 Oct 2020 10:12:07 +0800 Jisheng Zhang wrote:

Quoted 5 lines · On Wed, 14 Oct 2020 22:44:50 +0300 Adrian Hunter wrote:
+ Chunyan
On 14/10/20 1:32 pm, Jisheng Zhang wrote:

Quoted 8 lines

Quoted 4 lines
Quoted 1 line
Auto CMD Auto Select can only be used when v4_mode is enabled.
The SDHCI spec. doesn't seem to say that. AFAICS it refers only to v4.1 not v4 mode.
4.10 defines the "Auto CMD Auto Select" mode, v4 mode bit in SDHCI_HOST_CONTROL2 selects V3 compatible or V4 compatible mode, I think the v4 here includes v4.0, v4.1 and v4.2, so if we want to use the mode we have to ensure v4_mode

So if we want to use the "Auto CMD Auto Select" mode, we have to ensure v4 mode is enabled.

Quoted 29 lines

is enabled.

thanks

Quoted 24 lines

Quoted 23 lines

Fixes 427b6514d095 ("mmc: sdhci: Add Auto CMD Auto Select support")

Signed-off-by Jisheng Zhang <[email protected]>

---
 drivers/mmc/host/sdhci.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

drivers/mmc/host/sdhci.c +2 -1

index 592a55a34b58..5e0ec5df4074 100644
@@ -1386,7 +1386,8 @@ static inline void sdhci_auto_cmd_select(struct sdhci_host *host,
       * Select' is recommended rather than use of 'Auto CMD12
       * Enable' or 'Auto CMD23 Enable'.
       */
-     if (host->version >= SDHCI_SPEC_410 && (use_cmd12 || use_cmd23)) {
+     if (host->version >= SDHCI_SPEC_410 && host->v4_mode &&
+         (use_cmd12 || use_cmd23)) {
              *mode |= SDHCI_TRNS_AUTO_SEL;
 
              ctrl2 = sdhci_readw(host, SDHCI_HOST_CONTROL2);

Adrian Hunter

maintainer

2020-10-15 05:57 UTC #7 lore raw

Re: [PATCH] mmc: sdhci: Use Auto CMD Auto Select only when v4_mode is true

Quoted 16 lines · Jisheng Zhang

Quoted 13 lines · On Thu, 15 Oct 2020 10:12:07 +0800 Jisheng Zhang wrote:
Quoted 9 lines · On Wed, 14 Oct 2020 22:44:50 +0300 Adrian Hunter wrote:
+ Chunyan
Quoted 1 line · Jisheng Zhang
Auto CMD Auto Select can only be used when v4_mode is enabled.
The SDHCI spec. doesn't seem to say that. AFAICS it refers only to v4.1 not v4 mode.

Quoted 6 lines

Quoted 3 lines
4.10 defines the "Auto CMD Auto Select" mode, v4 mode bit in SDHCI_HOST_CONTROL2 selects V3 compatible or V4 compatible mode, I think the v4 here includes v4.0, v4.1 and v4.2, so if we want to use the mode we have to ensure v4_mode
So if we want to use the "Auto CMD Auto Select" mode, we have to ensure v4 mode is enabled.

But that is not exactly what the SDHCI spec. says. It is quite explicit about which registers and bit fields are affected by "Host Version 4 Enable =1".

So the question is whether this is standard or a quirk of your controller.

Quoted 31 lines

Quoted 30 lines

is enabled.

thanks

Quoted 24 lines

Quoted 23 lines

Fixes 427b6514d095 ("mmc: sdhci: Add Auto CMD Auto Select support")

Signed-off-by Jisheng Zhang <[email protected]>

---
 drivers/mmc/host/sdhci.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

drivers/mmc/host/sdhci.c +2 -1

index 592a55a34b58..5e0ec5df4074 100644
@@ -1386,7 +1386,8 @@ static inline void sdhci_auto_cmd_select(struct sdhci_host *host,
       * Select' is recommended rather than use of 'Auto CMD12
       * Enable' or 'Auto CMD23 Enable'.
       */
-     if (host->version >= SDHCI_SPEC_410 && (use_cmd12 || use_cmd23)) {
+     if (host->version >= SDHCI_SPEC_410 && host->v4_mode &&
+         (use_cmd12 || use_cmd23)) {
              *mode |= SDHCI_TRNS_AUTO_SEL;
 
              ctrl2 = sdhci_readw(host, SDHCI_HOST_CONTROL2);

Jisheng Zhang

author

2020-10-15 06:25 UTC #8 lore raw

Re: [PATCH] mmc: sdhci: Use Auto CMD Auto Select only when v4_mode is true

Quoted 23 lines · On Thu, 15 Oct 2020 08:57:05 +0300 Adrian Hunter wrote:

Quoted 20 lines · Jisheng Zhang
Quoted 16 lines · On Thu, 15 Oct 2020 10:12:07 +0800 Jisheng Zhang wrote:
Quoted 9 lines · On Wed, 14 Oct 2020 22:44:50 +0300 Adrian Hunter wrote:
+ Chunyan
Quoted 1 line · Jisheng Zhang
Auto CMD Auto Select can only be used when v4_mode is enabled.
The SDHCI spec. doesn't seem to say that. AFAICS it refers only to v4.1 not v4 mode.
4.10 defines the "Auto CMD Auto Select" mode, v4 mode bit in SDHCI_HOST_CONTROL2 selects V3 compatible or V4 compatible mode, I think the v4 here includes v4.0, v4.1 and v4.2, so if we want to use the mode we have to ensure v4_mode

Quoted 6 lines

Quoted 2 lines
So if we want to use the "Auto CMD Auto Select" mode, we have to ensure v4 mode is enabled.
But that is not exactly what the SDHCI spec. says. It is quite explicit about which registers and bit fields are affected by "Host Version 4 Enable =1".

Just my humble opinion, this is implied, my logic look like:

Host Version 4 Enable == 0 => only V3 compatible mode \
=> v4 mode is must for auto cmd auto select
No "Auto CMD Auto Select" definition in v3 spec /

Quoted 2 lines

So the question is whether this is standard or a quirk of your controller.

other v4 controllers can do the same benchmark test after removing sdhci_enable_v4_mode() in the controller's probe.

Thanks

Adrian Hunter

maintainer

2020-10-15 08:50 UTC #9 lore raw

Re: [PATCH] mmc: sdhci: Use Auto CMD Auto Select only when v4_mode is true

Quoted 32 lines · Jisheng Zhang

Quoted 23 lines · On Thu, 15 Oct 2020 08:57:05 +0300 Adrian Hunter wrote:
Quoted 20 lines · Jisheng Zhang
Quoted 16 lines · On Thu, 15 Oct 2020 10:12:07 +0800 Jisheng Zhang wrote:
Quoted 9 lines · On Wed, 14 Oct 2020 22:44:50 +0300 Adrian Hunter wrote:
+ Chunyan
Quoted 1 line · Jisheng Zhang
Auto CMD Auto Select can only be used when v4_mode is enabled.
The SDHCI spec. doesn't seem to say that. AFAICS it refers only to v4.1 not v4 mode.
4.10 defines the "Auto CMD Auto Select" mode, v4 mode bit in SDHCI_HOST_CONTROL2 selects V3 compatible or V4 compatible mode, I think the v4 here includes v4.0, v4.1 and v4.2, so if we want to use the mode we have to ensure v4_mode
Quoted 6 lines
Quoted 2 lines
So if we want to use the "Auto CMD Auto Select" mode, we have to ensure v4 mode is enabled.
But that is not exactly what the SDHCI spec. says. It is quite explicit about which registers and bit fields are affected by "Host Version 4 Enable =1".

Quoted 5 lines

Just my humble opinion, this is implied, my logic look like:
Host Version 4 Enable == 0 => only V3 compatible mode \
=> v4 mode is must for auto cmd auto select
No "Auto CMD Auto Select" definition in v3 spec /

Ok, we will need the commit message to explain the performance degradation, and which driver / version / platform, and a comment in the code explaining we require Version 4 Mode for Auto CMD Auto Select because some controllers expect it that way.

Jisheng Zhang

author

2020-10-14 10:42 UTC #10 lore raw

In reply to Adrian Hunter

Quoted 15 lines · On Tue, 13 Oct 2020 16:23:36 +0300 Adrian Hunter wrote:

Quoted 13 lines · Ulf Hansson
+ Adrian
Quoted 10 lines · Jisheng Zhang
Hi,
I found an emmc performance regression with below simple benchmark cmd:
dd if=/dev/mmcblk0 of=/dev/null bs=8192 count=100000
This regression has been bisected to commit 427b6514d0953bf (“mmc: sdhci: Add Auto CMD Auto Select support”)

Quoted 6 lines

Quoted 6 lines
Quoted 3 lines
If I revert this commit, the performance is good now.
I’m not sure whether this is common issue or not.
Can you provide some data, like how much is the regression and how many tests did you run?

before the commit, 120MB/s
after the commit, 51.3MB/s

tested 10 times

Quoted 4 lines

Quoted 2 lines
Did you try different block sizes of the dd command?
Also what is the SDHCI version? The patch in question is for version >= 4.1

V4.2

Quoted 2 lines

What driver?

sdhci-of-dwcmshc.c, but some phy tuning code isn't mainlined yet.

Quoted 2 lines

Can you tell if the patch changed which auto command is used?

autocmd23

Now I have a fix, the Auto CMD auto Select can only be used when v4 mode is enabled. Although the HW supports v4.1, but drivers may haven't enabled it due to various reasons. I just sent out the fix a few minutes ago.

Thanks