RE: [PATCH] mmc: sdhci-pci: Remove D3 delays for Intel BYT-related host controllers

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> -----Original Message-----
> From: Bjorn Helgaas [mailto:helgaas@xxxxxxxxxx]
> Sent: Monday, October 9, 2017 4:42 PM
> To: Hunter, Adrian <adrian.hunter@xxxxxxxxx>
> Cc: Ulf Hansson <ulf.hansson@xxxxxxxxxx>; linux-mmc <linux-
> mmc@xxxxxxxxxxxxxxx>; linux-pci <linux-pci@xxxxxxxxxxxxxxx>; Bjorn Helgaas
> <bhelgaas@xxxxxxxxxx>; linux-acpi <linux-acpi@xxxxxxxxxxxxxxx>; Rafael J.
> Wysocki <rjw@xxxxxxxxxxxxx>; Alan Cox <alan@xxxxxxxxxxxxxxx>
> Subject: Re: [PATCH] mmc: sdhci-pci: Remove D3 delays for Intel BYT-related
> host controllers
> 
> [+cc Alan]
> 
> On Mon, Oct 09, 2017 at 11:56:43AM +0300, Adrian Hunter wrote:
> > On 06/10/17 19:20, Bjorn Helgaas wrote:
> > > On Fri, Oct 06, 2017 at 03:50:59PM +0300, Adrian Hunter wrote:
> > >> The default D3 cold delay of 100 ms can cause pauses when streaming
> > >> audio from eMMC.
> > >>
> > >> Intel BYT-related host controllers do not need PCI D3 delays.
> > >> Although the delays can be set to zero via an ACPI _DSM,
> > >> unfortunately that is not being used in all cases. So just set the delays to
> zero in the driver.
> > >>
> > >> Signed-off-by: Adrian Hunter <adrian.hunter@xxxxxxxxx>
> > >> ---
> > >>  drivers/mmc/host/sdhci-pci-core.c | 17 +++++++++++++----
> > >>  1 file changed, 13 insertions(+), 4 deletions(-)
> > >>
> > >> diff --git a/drivers/mmc/host/sdhci-pci-core.c
> > >> b/drivers/mmc/host/sdhci-pci-core.c
> > >> index 5f3f7b51299f..14ef99b6e5b7 100644
> > >> --- a/drivers/mmc/host/sdhci-pci-core.c
> > >> +++ b/drivers/mmc/host/sdhci-pci-core.c
> > >> @@ -592,9 +592,18 @@ static void byt_read_dsm(struct sdhci_pci_slot
> *slot)
> > >>  	slot->chip->rpm_retune = intel_host->d3_retune;  }
> > >>
> > >> -static int byt_emmc_probe_slot(struct sdhci_pci_slot *slot)
> > >> +static void byt_common_setup(struct sdhci_pci_slot *slot)
> > >>  {
> > >>  	byt_read_dsm(slot);
> > >> +
> > >> +	/* PCI D3 delays are not needed */
> > >> +	slot->chip->pdev->d3_delay = 0;
> > >> +	slot->chip->pdev->d3cold_delay = 0;
> > >
> > > The fact that it doesn't need D3 delays sounds like a property of
> > > the device itself, regardless of which (if any) driver claims it.
> > >
> > > Can this be done as a quirk instead, maybe something in
> > > arch/x86/pci/fixup.c?  Maybe quirk_remove_d3_delay() could be used
> > > directly, or something like pci_d3_delay_fixup()?
> >
> > A quirk would work, but that would mean setting the quirk for each
> > device
> > (28 so far) and then, when we have new ids, adding them there and to
> > the driver, so it is more convenient just to do it in the driver.
> > Certainly, this is the only driver we have for these Intel SDHCI PCI host
> controllers.
> 
> pci_d3_delay_fixup() has code that mitigates the problem of adding new IDs.
> Maybe that quirk should simply be moved to arch/x86/pci/fixup.c?

pci_d3_delay_fixup() is not setting zero.  quirk_remove_d3_delay() and creating
a quirk_remove_d3cold_delay() would work.

> If we put this in the driver, won't we have the unnecessary delays on
> systems where the driver isn't in use?

The issue is unacceptable I/O latency (streaming audio through the device
was the example) which means you have to have a driver.

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux IBM ACPI]     [Linux Power Management]     [Linux Kernel]     [Linux Laptop]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux