* [PATCH] mmc: sdhci: remove mdelay in eMMC tuning
@ 2014-05-07 1:52 Nick Sanders
2014-05-07 8:35 ` Ulf Hansson
0 siblings, 1 reply; 7+ messages in thread
From: Nick Sanders @ 2014-05-07 1:52 UTC (permalink / raw)
To: Chris Ball
Cc: linux-mmc, Grant Grundler, Doug Anderson, Ulf Hansson,
Chang-Eun Choi-SSI, Nick Sanders
This patch removes an unneccesary 1ms mdelay in the HS200 tuning
loop, called 40 times per retuning. Currently this causes a latency
of >40ms on any emmc accesses triggering wake from runtime PM,
which can occur for a significant portion of reads on a mostly idle system.
The delay is left in place for SD Cards, which use
MMC_SEND_TUNING_BLOCK rather than MMC_SEND_TUNING_BLOCK_HS200.
I'm not able to find evidence that this is required for SD in the
specs I have access to, however this delay has been present from
initial checkin for SD so I have preserved the original behavior for
compatibility.
This has been verified to fix observed glitching on local audio
playback and recording on apps with inbuilt assumptions on storage
latency.
Signed-off-by: Nick Sanders <nsanders@chromium.org>
Reviewed-by: Grant Grundler <grundler@chromium.org>
Reviewed-by: Doug Anderson <dianders@chromium.org>
---
drivers/mmc/host/sdhci.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index df6d85a..05ffac2 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1987,7 +1987,10 @@ static int sdhci_execute_tuning(struct mmc_host *mmc, u32 opcode)
ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2);
tuning_loop_counter--;
timeout--;
- mdelay(1);
+
+ /* eMMC spec does not require a delay between tuning cycles */
+ if (opcode == MMC_SEND_TUNING_BLOCK)
+ mdelay(1);
} while (ctrl & SDHCI_CTRL_EXEC_TUNING);
/*
--
1.9.1.423.g4596e3a
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] mmc: sdhci: remove mdelay in eMMC tuning
2014-05-07 1:52 [PATCH] mmc: sdhci: remove mdelay in eMMC tuning Nick Sanders
@ 2014-05-07 8:35 ` Ulf Hansson
2014-05-07 22:01 ` Nick Sanders
2014-05-14 1:13 ` Grant Grundler
0 siblings, 2 replies; 7+ messages in thread
From: Ulf Hansson @ 2014-05-07 8:35 UTC (permalink / raw)
To: Nick Sanders
Cc: Chris Ball, linux-mmc, Grant Grundler, Doug Anderson, Chang-Eun Choi-SSI
On 7 May 2014 03:52, Nick Sanders <nsanders@chromium.org> wrote:
> This patch removes an unneccesary 1ms mdelay in the HS200 tuning
> loop, called 40 times per retuning. Currently this causes a latency
> of >40ms on any emmc accesses triggering wake from runtime PM,
> which can occur for a significant portion of reads on a mostly idle system.
Aha, so you are actually using "MMC_CAP_RUNTIME_RESUME" here - cool :-).
I suppose those patches that enables the "cap" has not reached
mainline yet. Any plans on sending them?
>
> The delay is left in place for SD Cards, which use
> MMC_SEND_TUNING_BLOCK rather than MMC_SEND_TUNING_BLOCK_HS200.
> I'm not able to find evidence that this is required for SD in the
> specs I have access to, however this delay has been present from
> initial checkin for SD so I have preserved the original behavior for
> compatibility.
>
> This has been verified to fix observed glitching on local audio
> playback and recording on apps with inbuilt assumptions on storage
> latency.
>
> Signed-off-by: Nick Sanders <nsanders@chromium.org>
> Reviewed-by: Grant Grundler <grundler@chromium.org>
> Reviewed-by: Doug Anderson <dianders@chromium.org>
Acked-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
> drivers/mmc/host/sdhci.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index df6d85a..05ffac2 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -1987,7 +1987,10 @@ static int sdhci_execute_tuning(struct mmc_host *mmc, u32 opcode)
> ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2);
> tuning_loop_counter--;
> timeout--;
> - mdelay(1);
> +
> + /* eMMC spec does not require a delay between tuning cycles */
> + if (opcode == MMC_SEND_TUNING_BLOCK)
> + mdelay(1);
> } while (ctrl & SDHCI_CTRL_EXEC_TUNING);
>
> /*
> --
> 1.9.1.423.g4596e3a
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] mmc: sdhci: remove mdelay in eMMC tuning
2014-05-07 8:35 ` Ulf Hansson
@ 2014-05-07 22:01 ` Nick Sanders
2014-05-14 1:13 ` Grant Grundler
1 sibling, 0 replies; 7+ messages in thread
From: Nick Sanders @ 2014-05-07 22:01 UTC (permalink / raw)
To: Ulf Hansson
Cc: Chris Ball, linux-mmc, Grant Grundler, Doug Anderson, Chang-Eun Choi-SSI
On Wed, May 7, 2014 at 1:35 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> Aha, so you are actually using "MMC_CAP_RUNTIME_RESUME" here - cool :-).
>
> I suppose those patches that enables the "cap" has not reached
> mainline yet. Any plans on sending them?
I am actually running 3.10, so MMC_CAP_RUNTIME_RESUME isn't used yet.
sdhci-acpi.c is enabling runtime pm for my device, and there are no
caps checks otherwise.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] mmc: sdhci: remove mdelay in eMMC tuning
2014-05-07 8:35 ` Ulf Hansson
2014-05-07 22:01 ` Nick Sanders
@ 2014-05-14 1:13 ` Grant Grundler
2014-05-14 1:22 ` Chris Ball
2014-05-14 12:18 ` Arnd Bergmann
1 sibling, 2 replies; 7+ messages in thread
From: Grant Grundler @ 2014-05-14 1:13 UTC (permalink / raw)
To: Ulf Hansson, Chris Ball
Cc: Nick Sanders, linux-mmc, Grant Grundler, Doug Anderson,
Chang-Eun Choi-SSI
On Wed, May 7, 2014 at 1:35 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> On 7 May 2014 03:52, Nick Sanders <nsanders@chromium.org> wrote:
>> This patch removes an unneccesary 1ms mdelay in the HS200 tuning
>> loop, called 40 times per retuning. Currently this causes a latency
>> of >40ms on any emmc accesses triggering wake from runtime PM,
>> which can occur for a significant portion of reads on a mostly idle system.
>
> Aha, so you are actually using "MMC_CAP_RUNTIME_RESUME" here - cool :-).
>
> I suppose those patches that enables the "cap" has not reached
> mainline yet. Any plans on sending them?
>
>>
>> The delay is left in place for SD Cards, which use
>> MMC_SEND_TUNING_BLOCK rather than MMC_SEND_TUNING_BLOCK_HS200.
>> I'm not able to find evidence that this is required for SD in the
>> specs I have access to, however this delay has been present from
>> initial checkin for SD so I have preserved the original behavior for
>> compatibility.
>>
>> This has been verified to fix observed glitching on local audio
>> playback and recording on apps with inbuilt assumptions on storage
>> latency.
>>
>> Signed-off-by: Nick Sanders <nsanders@chromium.org>
>> Reviewed-by: Grant Grundler <grundler@chromium.org>
>> Reviewed-by: Doug Anderson <dianders@chromium.org>
>
> Acked-by: Ulf Hansson <ulf.hansson@linaro.org>
Ulf,
Is your ACK enough to get this to land upstream or does cjb need to
respond it's been accepted?
It doesn't appear to have landed in mmc-next branch yet:
http://git.kernel.org/cgit/linux/kernel/git/cjb/mmc.git/log/?h=mmc-next
apologies if it appears I am being impatient...but it's been almost a week now.
thanks,
grant
>
>> ---
>> drivers/mmc/host/sdhci.c | 5 ++++-
>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>> index df6d85a..05ffac2 100644
>> --- a/drivers/mmc/host/sdhci.c
>> +++ b/drivers/mmc/host/sdhci.c
>> @@ -1987,7 +1987,10 @@ static int sdhci_execute_tuning(struct mmc_host *mmc, u32 opcode)
>> ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2);
>> tuning_loop_counter--;
>> timeout--;
>> - mdelay(1);
>> +
>> + /* eMMC spec does not require a delay between tuning cycles */
>> + if (opcode == MMC_SEND_TUNING_BLOCK)
>> + mdelay(1);
>> } while (ctrl & SDHCI_CTRL_EXEC_TUNING);
>>
>> /*
>> --
>> 1.9.1.423.g4596e3a
>>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] mmc: sdhci: remove mdelay in eMMC tuning
2014-05-14 1:13 ` Grant Grundler
@ 2014-05-14 1:22 ` Chris Ball
2014-05-14 12:18 ` Arnd Bergmann
1 sibling, 0 replies; 7+ messages in thread
From: Chris Ball @ 2014-05-14 1:22 UTC (permalink / raw)
To: Grant Grundler
Cc: Ulf Hansson, Nick Sanders, linux-mmc, Doug Anderson, Chang-Eun Choi-SSI
Hi,
On Wed, May 14 2014, Grant Grundler wrote:
>>> The delay is left in place for SD Cards, which use
>>> MMC_SEND_TUNING_BLOCK rather than MMC_SEND_TUNING_BLOCK_HS200.
>>> I'm not able to find evidence that this is required for SD in the
>>> specs I have access to, however this delay has been present from
>>> initial checkin for SD so I have preserved the original behavior for
>>> compatibility.
>>>
>>> This has been verified to fix observed glitching on local audio
>>> playback and recording on apps with inbuilt assumptions on storage
>>> latency.
>>>
>>> Signed-off-by: Nick Sanders <nsanders@chromium.org>
>>> Reviewed-by: Grant Grundler <grundler@chromium.org>
>>> Reviewed-by: Doug Anderson <dianders@chromium.org>
>>
>> Acked-by: Ulf Hansson <ulf.hansson@linaro.org>
>
> Ulf,
> Is your ACK enough to get this to land upstream or does cjb need to
> respond it's been accepted?
>
> It doesn't appear to have landed in mmc-next branch yet:
> http://git.kernel.org/cgit/linux/kernel/git/cjb/mmc.git/log/?h=mmc-next
>
> apologies if it appears I am being impatient...but it's been almost a week now.
Sorry about that. Looks fine, pushed to mmc-next with Ulf's ACK now.
- Chris.
--
Chris Ball <http://printf.net/>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] mmc: sdhci: remove mdelay in eMMC tuning
2014-05-14 1:13 ` Grant Grundler
2014-05-14 1:22 ` Chris Ball
@ 2014-05-14 12:18 ` Arnd Bergmann
2014-05-14 15:08 ` Doug Anderson
1 sibling, 1 reply; 7+ messages in thread
From: Arnd Bergmann @ 2014-05-14 12:18 UTC (permalink / raw)
To: Grant Grundler
Cc: Ulf Hansson, Chris Ball, Nick Sanders, linux-mmc, Doug Anderson,
Chang-Eun Choi-SSI
On Tuesday 13 May 2014 18:13:30 Grant Grundler wrote:
> On Wed, May 7, 2014 at 1:35 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> > On 7 May 2014 03:52, Nick Sanders <nsanders@chromium.org> wrote:
> >> This patch removes an unneccesary 1ms mdelay in the HS200 tuning
> >> loop, called 40 times per retuning. Currently this causes a latency
> >> of >40ms on any emmc accesses triggering wake from runtime PM,
> >> which can occur for a significant portion of reads on a mostly idle system.
> >
> > Aha, so you are actually using "MMC_CAP_RUNTIME_RESUME" here - cool :-).
> >
> > I suppose those patches that enables the "cap" has not reached
> > mainline yet. Any plans on sending them?
> >
> >>
> >> The delay is left in place for SD Cards, which use
> >> MMC_SEND_TUNING_BLOCK rather than MMC_SEND_TUNING_BLOCK_HS200.
> >> I'm not able to find evidence that this is required for SD in the
> >> specs I have access to, however this delay has been present from
> >> initial checkin for SD so I have preserved the original behavior for
> >> compatibility.
> >>
> >> This has been verified to fix observed glitching on local audio
> >> playback and recording on apps with inbuilt assumptions on storage
> >> latency.
> >>
> >> Signed-off-by: Nick Sanders <nsanders@chromium.org>
> >> Reviewed-by: Grant Grundler <grundler@chromium.org>
> >> Reviewed-by: Doug Anderson <dianders@chromium.org>
> >
> > Acked-by: Ulf Hansson <ulf.hansson@linaro.org>
>
> Ulf,
> Is your ACK enough to get this to land upstream or does cjb need to
> respond it's been accepted?
>
> It doesn't appear to have landed in mmc-next branch yet:
> http://git.kernel.org/cgit/linux/kernel/git/cjb/mmc.git/log/?h=mmc-next
>
> apologies if it appears I am being impatient...but it's been almost a week now.
>
I just stumbled over this thread. Looking at the callers of the
execute_tuning() function, I see that all three come from non-atomic
context. How about an add-on patch to turn the mdelay(1) into an
msleep(1)? That would be much friendlier to other tasks running
concurrently.
Arnd
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] mmc: sdhci: remove mdelay in eMMC tuning
2014-05-14 12:18 ` Arnd Bergmann
@ 2014-05-14 15:08 ` Doug Anderson
0 siblings, 0 replies; 7+ messages in thread
From: Doug Anderson @ 2014-05-14 15:08 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Grant Grundler, Ulf Hansson, Chris Ball, Nick Sanders, linux-mmc,
Chang-Eun Choi-SSI
Arnd,
On Wed, May 14, 2014 at 5:18 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> I just stumbled over this thread. Looking at the callers of the
> execute_tuning() function, I see that all three come from non-atomic
> context. How about an add-on patch to turn the mdelay(1) into an
> msleep(1)? That would be much friendlier to other tasks running
> concurrently.
Even better if someone could tell us why the mdelay() was there to
begin with. As I understand it Nick was confident that it wasn't
needed for eMMC. He didn't _think_ it was needed for SD cards either
but it was safer to leave it there for SD cards since the delay wasn't
causing any known problems on them.
-Doug
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2014-05-14 15:08 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-05-07 1:52 [PATCH] mmc: sdhci: remove mdelay in eMMC tuning Nick Sanders
2014-05-07 8:35 ` Ulf Hansson
2014-05-07 22:01 ` Nick Sanders
2014-05-14 1:13 ` Grant Grundler
2014-05-14 1:22 ` Chris Ball
2014-05-14 12:18 ` Arnd Bergmann
2014-05-14 15:08 ` Doug Anderson
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.