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

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

[Adrian Hunter]

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@intel.com>
---
 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;
+}
+
+static int byt_emmc_probe_slot(struct sdhci_pci_slot *slot)
+{
+	byt_common_setup(slot);
 	slot->host->mmc->caps |= MMC_CAP_8_BIT_DATA | MMC_CAP_NONREMOVABLE |
 				 MMC_CAP_HW_RESET | MMC_CAP_1_8V_DDR |
 				 MMC_CAP_CMD_DURING_TFR |
@@ -649,7 +658,7 @@ static int ni_byt_sdio_probe_slot(struct sdhci_pci_slot *slot)
 {
 	int err;
 
-	byt_read_dsm(slot);
+	byt_common_setup(slot);
 
 	err = ni_set_max_freq(slot);
 	if (err)
@@ -662,7 +671,7 @@ static int ni_byt_sdio_probe_slot(struct sdhci_pci_slot *slot)
 
 static int byt_sdio_probe_slot(struct sdhci_pci_slot *slot)
 {
-	byt_read_dsm(slot);
+	byt_common_setup(slot);
 	slot->host->mmc->caps |= MMC_CAP_POWER_OFF_CARD | MMC_CAP_NONREMOVABLE |
 				 MMC_CAP_WAIT_WHILE_BUSY;
 	return 0;
@@ -670,7 +679,7 @@ static int byt_sdio_probe_slot(struct sdhci_pci_slot *slot)
 
 static int byt_sd_probe_slot(struct sdhci_pci_slot *slot)
 {
-	byt_read_dsm(slot);
+	byt_common_setup(slot);
 	slot->host->mmc->caps |= MMC_CAP_WAIT_WHILE_BUSY |
 				 MMC_CAP_AGGRESSIVE_PM | MMC_CAP_CD_WAKE;
 	slot->cd_idx = 0;
-- 
1.9.1

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

[Bjorn Helgaas]

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@intel.com>
> ---
>  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()?

> +}
> +
> +static int byt_emmc_probe_slot(struct sdhci_pci_slot *slot)
> +{
> +	byt_common_setup(slot);
>  	slot->host->mmc->caps |= MMC_CAP_8_BIT_DATA | MMC_CAP_NONREMOVABLE |
>  				 MMC_CAP_HW_RESET | MMC_CAP_1_8V_DDR |
>  				 MMC_CAP_CMD_DURING_TFR |
> @@ -649,7 +658,7 @@ static int ni_byt_sdio_probe_slot(struct sdhci_pci_slot *slot)
>  {
>  	int err;
>  
> -	byt_read_dsm(slot);
> +	byt_common_setup(slot);
>  
>  	err = ni_set_max_freq(slot);
>  	if (err)
> @@ -662,7 +671,7 @@ static int ni_byt_sdio_probe_slot(struct sdhci_pci_slot *slot)
>  
>  static int byt_sdio_probe_slot(struct sdhci_pci_slot *slot)
>  {
> -	byt_read_dsm(slot);
> +	byt_common_setup(slot);
>  	slot->host->mmc->caps |= MMC_CAP_POWER_OFF_CARD | MMC_CAP_NONREMOVABLE |
>  				 MMC_CAP_WAIT_WHILE_BUSY;
>  	return 0;
> @@ -670,7 +679,7 @@ static int byt_sdio_probe_slot(struct sdhci_pci_slot *slot)
>  
>  static int byt_sd_probe_slot(struct sdhci_pci_slot *slot)
>  {
> -	byt_read_dsm(slot);
> +	byt_common_setup(slot);
>  	slot->host->mmc->caps |= MMC_CAP_WAIT_WHILE_BUSY |
>  				 MMC_CAP_AGGRESSIVE_PM | MMC_CAP_CD_WAKE;
>  	slot->cd_idx = 0;
> -- 
> 1.9.1
> 

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

[Adrian Hunter]

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@intel.com>
>> ---
>>  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.

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

[Bjorn Helgaas]

[+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@intel.com>
> >> ---
> >>  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?

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

Bjorn

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

[Hunter, Adrian]

> -----Original Message-----
> From: Bjorn Helgaas [mailto:helgaas@kernel.org]
> Sent: Monday, October 9, 2017 4:42 PM
> To: Hunter, Adrian <adrian.hunter@intel.com>
> Cc: Ulf Hansson <ulf.hansson@linaro.org>; linux-mmc <linux-
> mmc@vger.kernel.org>; linux-pci <linux-pci@vger.kernel.org>; Bjorn Helgaas
> <bhelgaas@google.com>; linux-acpi <linux-acpi@vger.kernel.org>; Rafael J.
> Wysocki <rjw@rjwysocki.net>; Alan Cox <alan@linux.intel.com>
> 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@intel.com>
> > >> ---
> > >>  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.

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

[Hunter, Adrian]

> -----Original Message-----
> From: Bjorn Helgaas [mailto:helgaas@kernel.org]
> Sent: Monday, October 9, 2017 4:42 PM
> To: Hunter, Adrian <adrian.hunter@intel.com>
> Cc: Ulf Hansson <ulf.hansson@linaro.org>; linux-mmc <linux-
> mmc@vger.kernel.org>; linux-pci <linux-pci@vger.kernel.org>; Bjorn Helgaa=
s
> <bhelgaas@google.com>; linux-acpi <linux-acpi@vger.kernel.org>; Rafael J.
> Wysocki <rjw@rjwysocki.net>; Alan Cox <alan@linux.intel.com>
> Subject: Re: [PATCH] mmc: sdhci-pci: Remove D3 delays for Intel BYT-relat=
ed
> host controllers
>=20
> [+cc Alan]
>=20
> 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 d=
elays to
> zero in the driver.
> > >>
> > >> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> > >> ---
> > >>  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 =3D 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 =3D 0;
> > >> +	slot->chip->pdev->d3cold_delay =3D 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 ho=
st
> controllers.
>=20
> pci_d3_delay_fixup() has code that mitigates the problem of adding new ID=
s.
> 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 crea=
ting
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.

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

[Bjorn Helgaas]

On Mon, Oct 09, 2017 at 08:01:06PM +0000, Hunter, Adrian wrote:
> > -----Original Message-----
> > From: Bjorn Helgaas [mailto:helgaas@kernel.org]
> > Sent: Monday, October 9, 2017 4:42 PM
> > To: Hunter, Adrian <adrian.hunter@intel.com>
> > Cc: Ulf Hansson <ulf.hansson@linaro.org>; linux-mmc <linux-
> > mmc@vger.kernel.org>; linux-pci <linux-pci@vger.kernel.org>; Bjorn Helgaas
> > <bhelgaas@google.com>; linux-acpi <linux-acpi@vger.kernel.org>; Rafael J.
> > Wysocki <rjw@rjwysocki.net>; Alan Cox <alan@linux.intel.com>
> > 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@intel.com>
> > > >> ---
> > > >>  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.

Your concern was that a quirk would require a long list of device IDs
and we'd have to add new ones.  I share that concern, and my point was
that pci_d3_delay_fixup() checks for the platform type and sets the
delay based on that, so it doesn't have a long list of device IDs.  

Whether the appropriate delay is zero or 3 is a question for you Intel
guys to sort out.  We already have:

  pci_d3delay_fixup()     # arch/x86/pci/intel_mid_pci.c
  pci_d3_delay_fixup()    # drivers/staging/media/atomisp/platform/intel-mid/intel_mid_pcihelpers.c
  atomisp_pci_probe()     # drivers/staging/media/atomisp/pci/atomisp2/atomisp_v4l2.c

and you're proposing to add a fourth one.  I think there's a lot of
overlap there, and it'd be nice to clean that up.

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

If I don't use audio, I don't need the driver.  My concern is that
even without the driver, there may be paths that use
pci_dev_d3_sleep() for this device, e.g., suspend/resume of the whole
system.  If that's the case, we should avoid the default 10ms delay
even if there's no driver loaded.

Maybe we never use pci_dev_d3_sleep() if there's no driver, but I
haven't been able to convince myself of that yet.

Bjorn

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

[Alan Cox]

> Your concern was that a quirk would require a long list of device IDs
> and we'd have to add new ones.  I share that concern,

I'm not sure I do. The bus topology tells you what is on die, and the
id of the root bridge tells you if it's one of the parts you can do
this.

Alan

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

[Alan Cox]

> Your concern was that a quirk would require a long list of device IDs
> and we'd have to add new ones.  I share that concern,

I'm not sure I do. The bus topology tells you what is on die, and the
id of the root bridge tells you if it's one of the parts you can do
this.

Alan

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

[Bjorn Helgaas]

On Tue, Oct 10, 2017 at 10:01:35AM +0100, Alan Cox wrote:
> > Your concern was that a quirk would require a long list of device IDs
> > and we'd have to add new ones.  I share that concern,
> 
> I'm not sure I do. The bus topology tells you what is on die, and the
> id of the root bridge tells you if it's one of the parts you can do
> this.

Your a49d25364dfb ("staging/atomisp: Add support for the Intel IPU
v2") added the most generic version (pci_d3_delay_fixup() uses

  DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, PCI_ANY_ID, pci_d3_delay_fixup);

so there's no list of device IDs.  If this is really a property of the
family (BYT/CHT/etc), it makes sense to me to do it that way, but if
you think it's better to have a list, that's OK too.

The more interesting question to me is whether the quirk should be in
an optional driver or in the always-present arch code.  My assumption
is that putting it in the driver means that if the driver isn't
enabled, we're doing unnecessary delays.

Bjorn