eMMC performance regression caused by 427b6514d0953bf

eMMC performance regression caused by 427b6514d0953bf

[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”)

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

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

Thanks

Re: eMMC performance regression caused by 427b6514d0953bf

[Ulf Hansson]

+ Adrian

On Tue, 13 Oct 2020 at 13:09, Jisheng Zhang <Jisheng.Zhang@synaptics.com> wrote:
>
> 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.

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

Re: eMMC performance regression caused by 427b6514d0953bf

[Adrian Hunter]

On 13/10/20 3:32 pm, Ulf Hansson wrote:
> + Adrian
> 
> On Tue, 13 Oct 2020 at 13:09, Jisheng Zhang <Jisheng.Zhang@synaptics.com> wrote:
>>
>> 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.
> 
> 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?

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

[Jisheng Zhang]

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 <Jisheng.Zhang@synaptics.com>
---
 drivers/mmc/host/sdhci.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 592a55a34b58..5e0ec5df4074 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -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);
-- 
2.28.0

Re: eMMC performance regression caused by 427b6514d0953bf

[Jisheng Zhang]

On Tue, 13 Oct 2020 16:23:36 +0300 Adrian Hunter wrote:

> 
> On 13/10/20 3:32 pm, Ulf Hansson wrote:
> > + Adrian
> >
> > On Tue, 13 Oct 2020 at 13:09, Jisheng Zhang <Jisheng.Zhang@synaptics.com> wrote:  
> >>
> >> 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.  
> >
> > 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

> >
> > 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

> 
> What driver?

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

> 
> 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

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

[Adrian Hunter]

+ Chunyan

On 14/10/20 1:32 pm, Jisheng Zhang wrote:
> 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.

> 
> Fixes: 427b6514d095 ("mmc: sdhci: Add Auto CMD Auto Select support")
> Signed-off-by: Jisheng Zhang <Jisheng.Zhang@synaptics.com>
> ---
>  drivers/mmc/host/sdhci.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index 592a55a34b58..5e0ec5df4074 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -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);
> 

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

[Jisheng Zhang]

On Wed, 14 Oct 2020 22:44:50 +0300 Adrian Hunter wrote:

>
> 
> + Chunyan
> 
> On 14/10/20 1:32 pm, Jisheng Zhang wrote:
> > 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

> 
> >
> > Fixes: 427b6514d095 ("mmc: sdhci: Add Auto CMD Auto Select support")
> > Signed-off-by: Jisheng Zhang <Jisheng.Zhang@synaptics.com>
> > ---
> >  drivers/mmc/host/sdhci.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> > index 592a55a34b58..5e0ec5df4074 100644
> > --- a/drivers/mmc/host/sdhci.c
> > +++ b/drivers/mmc/host/sdhci.c
> > @@ -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);
> >  
> 

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

[Jisheng Zhang]

On Thu, 15 Oct 2020 10:12:07 +0800 Jisheng Zhang wrote:


> 
> On Wed, 14 Oct 2020 22:44:50 +0300 Adrian Hunter wrote:
> 
> >
> >
> > + Chunyan
> >
> > On 14/10/20 1:32 pm, Jisheng Zhang wrote:  
> > > 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.

> is enabled.
> 
> thanks
> 
> >  
> > >
> > > Fixes: 427b6514d095 ("mmc: sdhci: Add Auto CMD Auto Select support")
> > > Signed-off-by: Jisheng Zhang <Jisheng.Zhang@synaptics.com>
> > > ---
> > >  drivers/mmc/host/sdhci.c | 3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> > > index 592a55a34b58..5e0ec5df4074 100644
> > > --- a/drivers/mmc/host/sdhci.c
> > > +++ b/drivers/mmc/host/sdhci.c
> > > @@ -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);
> > >  
> >  
> 

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

[Adrian Hunter]

On 15/10/20 5:38 am, Jisheng Zhang wrote:
> On Thu, 15 Oct 2020 10:12:07 +0800 Jisheng Zhang wrote:
> 
> 
>>
>> On Wed, 14 Oct 2020 22:44:50 +0300 Adrian Hunter wrote:
>>
>>>
>>>
>>> + Chunyan
>>>
>>> On 14/10/20 1:32 pm, Jisheng Zhang wrote:  
>>>> 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.

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.

> 
>> is enabled.
>>
>> thanks
>>
>>>  
>>>>
>>>> Fixes: 427b6514d095 ("mmc: sdhci: Add Auto CMD Auto Select support")
>>>> Signed-off-by: Jisheng Zhang <Jisheng.Zhang@synaptics.com>
>>>> ---
>>>>  drivers/mmc/host/sdhci.c | 3 ++-
>>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>>>> index 592a55a34b58..5e0ec5df4074 100644
>>>> --- a/drivers/mmc/host/sdhci.c
>>>> +++ b/drivers/mmc/host/sdhci.c
>>>> @@ -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);
>>>>  
>>>  
>>
> 

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

[Jisheng Zhang]

On Thu, 15 Oct 2020 08:57:05 +0300 Adrian Hunter wrote:

> 
> 
> On 15/10/20 5:38 am, Jisheng Zhang wrote:
> > On Thu, 15 Oct 2020 10:12:07 +0800 Jisheng Zhang wrote:
> >
> >  
> >>
> >> On Wed, 14 Oct 2020 22:44:50 +0300 Adrian Hunter wrote:
> >>  
> >>>
> >>>
> >>> + Chunyan
> >>>
> >>> On 14/10/20 1:32 pm, Jisheng Zhang wrote:  
> >>>> 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.  
> 
> 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      /

> 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

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

[Adrian Hunter]

On 15/10/20 9:24 am, Jisheng Zhang wrote:
> On Thu, 15 Oct 2020 08:57:05 +0300 Adrian Hunter wrote:
> 
>>
>>
>> On 15/10/20 5:38 am, Jisheng Zhang wrote:
>>> On Thu, 15 Oct 2020 10:12:07 +0800 Jisheng Zhang wrote:
>>>
>>>  
>>>>
>>>> On Wed, 14 Oct 2020 22:44:50 +0300 Adrian Hunter wrote:
>>>>  
>>>>>
>>>>>
>>>>> + Chunyan
>>>>>
>>>>> On 14/10/20 1:32 pm, Jisheng Zhang wrote:  
>>>>>> 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.  
>>
>> 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      /

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.