[PATCH v1] PCI/PM: Switch D3Hot delay to use usleep_range

[PATCH v1] PCI/PM: Switch D3Hot delay to use usleep_range

[Will McVicker]

From: Sajid Dalvi <sdalvi@google.com>

Since the PCI spec requires a 10ms D3Hot delay (defined by
PCI_PM_D3HOT_WAIT) and a few of the PCI quirks update the D3Hot delay up
to 20ms, let's switch from msleep to usleep_range to improve the delay
accuracy.

This patch came from Sajid Dalvi <sdalvi@google.com> in the Pixel 6
kernel tree [1]. Testing on a Pixel 6, found that the 10ms delay for
the Exynos PCIe device was on average delaying for 19ms when the spec
requires 10ms. Switching from msleep to uslseep_delay therefore
decreases the resume time on a Pixel 6 on average by 9ms.

[1] https://android.googlesource.com/kernel/gs/+/18a8cad68d8e6d50f339a716a18295e6d987cee3

Signed-off-by: Sajid Dalvi <sdalvi@google.com>
Signed-off-by: Will McVicker <willmcvicker@google.com>
---
 drivers/pci/pci.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 95bc329e74c0..5ae5b3c4dc9b 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -72,7 +72,8 @@ static void pci_dev_d3_sleep(struct pci_dev *dev)
 		delay = pci_pm_d3hot_delay;
 
 	if (delay)
-		msleep(delay);
+		usleep_range(delay * USEC_PER_MSEC,
+			     (delay + 2) * USEC_PER_MSEC);
 }
 
 bool pci_reset_supported(struct pci_dev *dev)

base-commit: 2ae08b36c06ea8df73a79f6b80ff7964e006e9e3
-- 
2.37.1.559.g78731f0fdb-goog

Re: [PATCH v1] PCI/PM: Switch D3Hot delay to use usleep_range

[Matthias Kaehlcke]

On Thu, Aug 11, 2022 at 06:40:01PM +0000, Will McVicker wrote:
> From: Sajid Dalvi <sdalvi@google.com>
> 
> Since the PCI spec requires a 10ms D3Hot delay (defined by
> PCI_PM_D3HOT_WAIT) and a few of the PCI quirks update the D3Hot delay up
> to 20ms, let's switch from msleep to usleep_range to improve the delay
> accuracy.
> 
> This patch came from Sajid Dalvi <sdalvi@google.com> in the Pixel 6
> kernel tree [1]. Testing on a Pixel 6, found that the 10ms delay for
> the Exynos PCIe device was on average delaying for 19ms when the spec
> requires 10ms. Switching from msleep to uslseep_delay therefore
> decreases the resume time on a Pixel 6 on average by 9ms.
> 
> [1] https://android.googlesource.com/kernel/gs/+/18a8cad68d8e6d50f339a716a18295e6d987cee3
> 
> Signed-off-by: Sajid Dalvi <sdalvi@google.com>
> Signed-off-by: Will McVicker <willmcvicker@google.com>
> ---
>  drivers/pci/pci.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 95bc329e74c0..5ae5b3c4dc9b 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -72,7 +72,8 @@ static void pci_dev_d3_sleep(struct pci_dev *dev)
>  		delay = pci_pm_d3hot_delay;
>  
>  	if (delay)
> -		msleep(delay);
> +		usleep_range(delay * USEC_PER_MSEC,
> +			     (delay + 2) * USEC_PER_MSEC);

You could also use fsleep(), which ends up calling usleep_range()
for (usecs > 10 && usecs <= 20000).

Re: [PATCH v1] PCI/PM: Switch D3Hot delay to use usleep_range

[William McVicker]

On 08/16/2022, Matthias Kaehlcke wrote:
> On Thu, Aug 11, 2022 at 06:40:01PM +0000, Will McVicker wrote:
> > From: Sajid Dalvi <sdalvi@google.com>
> > 
> > Since the PCI spec requires a 10ms D3Hot delay (defined by
> > PCI_PM_D3HOT_WAIT) and a few of the PCI quirks update the D3Hot delay up
> > to 20ms, let's switch from msleep to usleep_range to improve the delay
> > accuracy.
> > 
> > This patch came from Sajid Dalvi <sdalvi@google.com> in the Pixel 6
> > kernel tree [1]. Testing on a Pixel 6, found that the 10ms delay for
> > the Exynos PCIe device was on average delaying for 19ms when the spec
> > requires 10ms. Switching from msleep to uslseep_delay therefore
> > decreases the resume time on a Pixel 6 on average by 9ms.
> > 
> > [1] https://android.googlesource.com/kernel/gs/+/18a8cad68d8e6d50f339a716a18295e6d987cee3
> > 
> > Signed-off-by: Sajid Dalvi <sdalvi@google.com>
> > Signed-off-by: Will McVicker <willmcvicker@google.com>
> > ---
> >  drivers/pci/pci.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> > index 95bc329e74c0..5ae5b3c4dc9b 100644
> > --- a/drivers/pci/pci.c
> > +++ b/drivers/pci/pci.c
> > @@ -72,7 +72,8 @@ static void pci_dev_d3_sleep(struct pci_dev *dev)
> >  		delay = pci_pm_d3hot_delay;
> >  
> >  	if (delay)
> > -		msleep(delay);
> > +		usleep_range(delay * USEC_PER_MSEC,
> > +			     (delay + 2) * USEC_PER_MSEC);
> 
> You could also use fsleep(), which ends up calling usleep_range()
> for (usecs > 10 && usecs <= 20000).

Thanks for the suggestion. I see fsleep() uses 2 * usec for the upper range
which I think is a bit much for this optimization. The documentation says
in the worse case an interrupt will be triggered for the upper bound, but
I'm not entirely sure how often that'd be. Thoughts?

--Will

Re: [PATCH v1] PCI/PM: Switch D3Hot delay to use usleep_range

[Matthias Kaehlcke]

On Tue, Aug 16, 2022 at 10:17:31PM +0000, William McVicker wrote:
> On 08/16/2022, Matthias Kaehlcke wrote:
> > On Thu, Aug 11, 2022 at 06:40:01PM +0000, Will McVicker wrote:
> > > From: Sajid Dalvi <sdalvi@google.com>
> > > 
> > > Since the PCI spec requires a 10ms D3Hot delay (defined by
> > > PCI_PM_D3HOT_WAIT) and a few of the PCI quirks update the D3Hot delay up
> > > to 20ms, let's switch from msleep to usleep_range to improve the delay
> > > accuracy.
> > > 
> > > This patch came from Sajid Dalvi <sdalvi@google.com> in the Pixel 6
> > > kernel tree [1]. Testing on a Pixel 6, found that the 10ms delay for
> > > the Exynos PCIe device was on average delaying for 19ms when the spec
> > > requires 10ms. Switching from msleep to uslseep_delay therefore
> > > decreases the resume time on a Pixel 6 on average by 9ms.
> > > 
> > > [1] https://android.googlesource.com/kernel/gs/+/18a8cad68d8e6d50f339a716a18295e6d987cee3
> > > 
> > > Signed-off-by: Sajid Dalvi <sdalvi@google.com>
> > > Signed-off-by: Will McVicker <willmcvicker@google.com>
> > > ---
> > >  drivers/pci/pci.c | 3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> > > index 95bc329e74c0..5ae5b3c4dc9b 100644
> > > --- a/drivers/pci/pci.c
> > > +++ b/drivers/pci/pci.c
> > > @@ -72,7 +72,8 @@ static void pci_dev_d3_sleep(struct pci_dev *dev)
> > >  		delay = pci_pm_d3hot_delay;
> > >  
> > >  	if (delay)
> > > -		msleep(delay);
> > > +		usleep_range(delay * USEC_PER_MSEC,
> > > +			     (delay + 2) * USEC_PER_MSEC);
> > 
> > You could also use fsleep(), which ends up calling usleep_range()
> > for (usecs > 10 && usecs <= 20000).
> 
> Thanks for the suggestion. I see fsleep() uses 2 * usec for the upper range
> which I think is a bit much for this optimization. The documentation says
> in the worse case an interrupt will be triggered for the upper bound, but
> I'm not entirely sure how often that'd be. Thoughts?

Ah, I misread 'delay + 2' in your patch as 'delay * 2', which would result in
the same. Agreed, in the spirit of your optimization it probably doesn't make
sense to use fsleep().

The range of 2us is really narroy, you could consider making it something
like 10 or 20% of 'delay'.

Re: [PATCH v1] PCI/PM: Switch D3Hot delay to use usleep_range

[William McVicker]

On 08/16/2022, Matthias Kaehlcke wrote:
> On Tue, Aug 16, 2022 at 10:17:31PM +0000, William McVicker wrote:
> > On 08/16/2022, Matthias Kaehlcke wrote:
> > > On Thu, Aug 11, 2022 at 06:40:01PM +0000, Will McVicker wrote:
> > > > From: Sajid Dalvi <sdalvi@google.com>
> > > > 
> > > > Since the PCI spec requires a 10ms D3Hot delay (defined by
> > > > PCI_PM_D3HOT_WAIT) and a few of the PCI quirks update the D3Hot delay up
> > > > to 20ms, let's switch from msleep to usleep_range to improve the delay
> > > > accuracy.
> > > > 
> > > > This patch came from Sajid Dalvi <sdalvi@google.com> in the Pixel 6
> > > > kernel tree [1]. Testing on a Pixel 6, found that the 10ms delay for
> > > > the Exynos PCIe device was on average delaying for 19ms when the spec
> > > > requires 10ms. Switching from msleep to uslseep_delay therefore
> > > > decreases the resume time on a Pixel 6 on average by 9ms.
> > > > 
> > > > [1] https://android.googlesource.com/kernel/gs/+/18a8cad68d8e6d50f339a716a18295e6d987cee3
> > > > 
> > > > Signed-off-by: Sajid Dalvi <sdalvi@google.com>
> > > > Signed-off-by: Will McVicker <willmcvicker@google.com>
> > > > ---
> > > >  drivers/pci/pci.c | 3 ++-
> > > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> > > > index 95bc329e74c0..5ae5b3c4dc9b 100644
> > > > --- a/drivers/pci/pci.c
> > > > +++ b/drivers/pci/pci.c
> > > > @@ -72,7 +72,8 @@ static void pci_dev_d3_sleep(struct pci_dev *dev)
> > > >  		delay = pci_pm_d3hot_delay;
> > > >  
> > > >  	if (delay)
> > > > -		msleep(delay);
> > > > +		usleep_range(delay * USEC_PER_MSEC,
> > > > +			     (delay + 2) * USEC_PER_MSEC);
> > > 
> > > You could also use fsleep(), which ends up calling usleep_range()
> > > for (usecs > 10 && usecs <= 20000).
> > 
> > Thanks for the suggestion. I see fsleep() uses 2 * usec for the upper range
> > which I think is a bit much for this optimization. The documentation says
> > in the worse case an interrupt will be triggered for the upper bound, but
> > I'm not entirely sure how often that'd be. Thoughts?
> 
> Ah, I misread 'delay + 2' in your patch as 'delay * 2', which would result in
> the same. Agreed, in the spirit of your optimization it probably doesn't make
> sense to use fsleep().
> 
> The range of 2us is really narroy, you could consider making it something
> like 10 or 20% of 'delay'.

The delay is actually 2ms which is 20% of the default 10ms delay. I took
a look at the PCI quirks again and see delays of 0s, 20ms, and 120ms. Due
to the 120ms delay for Intel, let me add a check to use msleep() when delay
is greather than 20ms. For <20ms, I'll use usleep_range with a 20-25% upper
bound.

Thanks,
Will