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