2020-07-07 20:19:51

by Doug Anderson

[permalink] [raw]
Subject: [PATCH 0/2] spi: spi-qcom-qspi: Avoid some per-transfer overhead


Not to be confused with the similar series I posed for the _other_
Qualcomm SPI controller (spi-geni-qcom) [1], this one avoids the
overhead on the Quad SPI controller.

It's based atop the current Qualcomm tree including Rajendra's ("spi:
spi-qcom-qspi: Use OPP API to set clk/perf state"). As discussed in
individual patches, these could ideally land through the Qualcomm tree
with Mark's Ack.

Measuring:
* Before OPP / Interconnect patches reading all flash takes: ~3.4 seconds
* After OPP / Interconnect patches reading all flash takes: ~4.7 seconds
* After this patch reading all flash takes: ~3.3 seconds

[1] https://lore.kernel.org/r/[email protected]
[2] https://lore.kernel.org/r/[email protected]


Douglas Anderson (2):
spi: spi-qcom-qspi: Avoid clock setting if not needed
spi: spi-qcom-qspi: Set an autosuspend delay of 250 ms

drivers/spi/spi-qcom-qspi.c | 45 ++++++++++++++++++++++++++++---------
1 file changed, 35 insertions(+), 10 deletions(-)

--
2.27.0.383.g050319c2ae-goog


2020-07-07 20:20:28

by Doug Anderson

[permalink] [raw]
Subject: [PATCH 2/2] spi: spi-qcom-qspi: Set an autosuspend delay of 250 ms

In commit cff80645d6d3 ("spi: spi-qcom-qspi: Add interconnect support")
the spi_geni_runtime_suspend() and spi_geni_runtime_resume()
became a bit slower. Measuring on my hardware I see numbers in the
hundreds of microseconds now.

Let's use autosuspend to help avoid some of the overhead. Now if
we're doing a bunch of transfers we won't need to be constantly
chruning.

The number 250 ms for the autosuspend delay was picked a bit
arbitrarily, so if someone has measurements showing a better value we
could easily change this.

Fixes: cff80645d6d3 ("spi: spi-qcom-qspi: Add interconnect support")
Signed-off-by: Douglas Anderson <[email protected]>
---
This patch could go through the SPI tree or land in the Qualcomm tree.
The patch it Fixes is currently in the Qualcomm tree so if it lands in
the main SPI tree there'd be a bit of a perf regression in the
Qualcomm tree until things merge together in mainline.

drivers/spi/spi-qcom-qspi.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/drivers/spi/spi-qcom-qspi.c b/drivers/spi/spi-qcom-qspi.c
index 322b88c22a86..6c39b23222b8 100644
--- a/drivers/spi/spi-qcom-qspi.c
+++ b/drivers/spi/spi-qcom-qspi.c
@@ -553,6 +553,8 @@ static int qcom_qspi_probe(struct platform_device *pdev)
goto exit_probe_master_put;
}

+ pm_runtime_use_autosuspend(dev);
+ pm_runtime_set_autosuspend_delay(dev, 250);
pm_runtime_enable(dev);

ret = spi_register_master(master);
--
2.27.0.383.g050319c2ae-goog

2020-07-08 08:40:53

by Rajendra Nayak

[permalink] [raw]
Subject: Re: [PATCH 0/2] spi: spi-qcom-qspi: Avoid some per-transfer overhead


On 7/8/2020 1:46 AM, Douglas Anderson wrote:
>
> Not to be confused with the similar series I posed for the _other_
> Qualcomm SPI controller (spi-geni-qcom) [1], this one avoids the
> overhead on the Quad SPI controller.
>
> It's based atop the current Qualcomm tree including Rajendra's ("spi:
> spi-qcom-qspi: Use OPP API to set clk/perf state"). As discussed in
> individual patches, these could ideally land through the Qualcomm tree
> with Mark's Ack.
>
> Measuring:
> * Before OPP / Interconnect patches reading all flash takes: ~3.4 seconds
> * After OPP / Interconnect patches reading all flash takes: ~4.7 seconds
> * After this patch reading all flash takes: ~3.3 seconds

Thanks Doug, I saw similar benefit on my setup with these patches. They do help
reduce the (unnecessary) additional overhead so it makes sense to merge these
along with the OPP/Interconnect patches in-order to avoid the regression.

Reviewed-by: Rajendra Nayak <[email protected]>
Tested-by: Rajendra Nayak <[email protected]>

>
> [1] https://lore.kernel.org/r/[email protected]
> [2] https://lore.kernel.org/r/[email protected]
>
>
> Douglas Anderson (2):
> spi: spi-qcom-qspi: Avoid clock setting if not needed
> spi: spi-qcom-qspi: Set an autosuspend delay of 250 ms
>
> drivers/spi/spi-qcom-qspi.c | 45 ++++++++++++++++++++++++++++---------
> 1 file changed, 35 insertions(+), 10 deletions(-)
>

--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

2020-07-08 17:37:46

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH 2/2] spi: spi-qcom-qspi: Set an autosuspend delay of 250 ms

On Tue, Jul 07, 2020 at 01:16:41PM -0700, Douglas Anderson wrote:
> In commit cff80645d6d3 ("spi: spi-qcom-qspi: Add interconnect support")
> the spi_geni_runtime_suspend() and spi_geni_runtime_resume()
> became a bit slower. Measuring on my hardware I see numbers in the
> hundreds of microseconds now.

Acked-by: Mark Brown <[email protected]>


Attachments:
(No filename) (355.00 B)
signature.asc (499.00 B)
Download all attachments

2020-07-09 06:55:19

by Mukesh, Savaliya

[permalink] [raw]
Subject: Re: [PATCH 0/2] spi: spi-qcom-qspi: Avoid some per-transfer overhead


On 7/8/2020 1:46 AM, Douglas Anderson wrote:
> Not to be confused with the similar series I posed for the _other_
> Qualcomm SPI controller (spi-geni-qcom) [1], this one avoids the
> overhead on the Quad SPI controller.
>
> It's based atop the current Qualcomm tree including Rajendra's ("spi:
> spi-qcom-qspi: Use OPP API to set clk/perf state"). As discussed in
> individual patches, these could ideally land through the Qualcomm tree
> with Mark's Ack.
>
> Measuring:
> * Before OPP / Interconnect patches reading all flash takes: ~3.4 seconds
> * After OPP / Interconnect patches reading all flash takes: ~4.7 seconds
> * After this patch reading all flash takes: ~3.3 seconds
>
> [1] https://lore.kernel.org/r/[email protected]
> [2] https://lore.kernel.org/r/[email protected]
>
>
> Douglas Anderson (2):
> spi: spi-qcom-qspi: Avoid clock setting if not needed
> spi: spi-qcom-qspi: Set an autosuspend delay of 250 ms
>
> drivers/spi/spi-qcom-qspi.c | 45 ++++++++++++++++++++++++++++---------
> 1 file changed, 35 insertions(+), 10 deletions(-)

Reviewed-by: Mukesh Kumar Savaliya <[email protected]>