* Re: [PATCH] ACPI / LPSS: Ignore 10ms delay for Braswell and Baytrail [not found] <1440090968-17728-1-git-send-email-srinidhi.kasagar@intel.com> @ 2015-08-20 12:38 ` Mika Westerberg 2015-08-20 13:04 ` Heikki Krogerus 2015-08-21 12:11 ` Kasagar, Srinidhi 0 siblings, 2 replies; 15+ messages in thread From: Mika Westerberg @ 2015-08-20 12:38 UTC (permalink / raw) To: Srinidhi Kasagar Cc: linux-acpi, rafael.j.wysocki, Kumar P Mahesh, Heikki Krogerus +Heikki On Thu, Aug 20, 2015 at 10:46:07PM +0530, Srinidhi Kasagar wrote: > LPSS devices in Braswell and Baytrail does not need the default > 10ms d3_delay imposed by PCI specification. Removing this > unnecessary delay significantly reduces the resume time > (~200ms on Braswell/Cherrytrail) on these platforms. > > Signed-off-by: Srinidhi Kasagar <srinidhi.kasagar@intel.com> > Signed-off-by: Kumar P Mahesh <mahesh.kumar.p@intel.com> Have you tested this on Asus T100? The delay was actually needed in order to restore the context IIRC. Also you are saying Braswell and Baytrail but... > --- > drivers/acpi/acpi_lpss.c | 59 ++++++++++++++++++++++++++++++++++++---------- > 1 file changed, 46 insertions(+), 13 deletions(-) > > diff --git a/drivers/acpi/acpi_lpss.c b/drivers/acpi/acpi_lpss.c > index 37fb19047603..9437b5fa59c3 100644 > --- a/drivers/acpi/acpi_lpss.c > +++ b/drivers/acpi/acpi_lpss.c > @@ -60,6 +60,7 @@ ACPI_MODULE_NAME("acpi_lpss"); > #define LPSS_CLK_DIVIDER BIT(2) > #define LPSS_LTR BIT(3) > #define LPSS_SAVE_CTX BIT(4) > +#define LPSS_NO_D3_DELAY BIT(5) > > struct lpss_private_data; > > @@ -134,6 +135,17 @@ static struct lpss_device_desc lpt_dev_desc = { > .prv_offset = 0x800, > }; > > +static struct lpss_device_desc bsw_lpt_dev_desc = { > + .flags = LPSS_CLK | LPSS_CLK_GATE | LPSS_CLK_DIVIDER | LPSS_LTR > + | LPSS_NO_D3_DELAY, > + .prv_offset = 0x800, > +}; > + > +static struct lpss_device_desc bsw_lpt_i2c_dev_desc = { > + .flags = LPSS_CLK | LPSS_CLK_GATE | LPSS_LTR | LPSS_NO_D3_DELAY, > + .prv_offset = 0x800, > +}; > + > static struct lpss_device_desc lpt_i2c_dev_desc = { > .flags = LPSS_CLK | LPSS_CLK_GATE | LPSS_LTR, > .prv_offset = 0x800, > @@ -146,18 +158,33 @@ static struct lpss_device_desc lpt_uart_dev_desc = { > .setup = lpss_uart_setup, > }; > > +static struct lpss_device_desc bsw_lpt_uart_dev_desc = { > + .flags = LPSS_CLK | LPSS_CLK_GATE | LPSS_CLK_DIVIDER | LPSS_LTR > + | LPSS_NO_D3_DELAY, > + .clk_con_id = "baudclk", > + .prv_offset = 0x800, > + .setup = lpss_uart_setup, > +}; > + > static struct lpss_device_desc lpt_sdio_dev_desc = { > .flags = LPSS_LTR, > .prv_offset = 0x1000, > .prv_size_override = 0x1018, > }; > > +static struct lpss_device_desc bsw_lpt_sdio_dev_desc = { > + .flags = LPSS_LTR | LPSS_NO_D3_DELAY, > + .prv_offset = 0x1000, > + .prv_size_override = 0x1018, > +}; > + > static struct lpss_device_desc byt_pwm_dev_desc = { > - .flags = LPSS_SAVE_CTX, > + .flags = LPSS_SAVE_CTX | LPSS_NO_D3_DELAY, > }; > > static struct lpss_device_desc byt_uart_dev_desc = { > - .flags = LPSS_CLK | LPSS_CLK_GATE | LPSS_CLK_DIVIDER | LPSS_SAVE_CTX, > + .flags = LPSS_CLK | LPSS_CLK_GATE | LPSS_CLK_DIVIDER | LPSS_SAVE_CTX > + | LPSS_NO_D3_DELAY, > .clk_con_id = "baudclk", > .prv_offset = 0x800, > .setup = lpss_uart_setup, > @@ -173,13 +200,14 @@ static struct lpss_device_desc byt_sdio_dev_desc = { > }; > > static struct lpss_device_desc byt_i2c_dev_desc = { > - .flags = LPSS_CLK | LPSS_SAVE_CTX, > + .flags = LPSS_CLK | LPSS_SAVE_CTX | LPSS_NO_D3_DELAY, > .prv_offset = 0x800, > .setup = byt_i2c_setup, > }; > > static struct lpss_device_desc bsw_spi_dev_desc = { > - .flags = LPSS_CLK | LPSS_CLK_GATE | LPSS_CLK_DIVIDER | LPSS_SAVE_CTX, > + .flags = LPSS_CLK | LPSS_CLK_GATE | LPSS_CLK_DIVIDER | LPSS_SAVE_CTX > + | LPSS_NO_D3_DELAY, > .prv_offset = 0x400, > .setup = lpss_deassert_reset, > }; > @@ -219,13 +247,13 @@ static const struct acpi_device_id acpi_lpss_device_ids[] = { > { "8086228E", LPSS_ADDR(bsw_spi_dev_desc) }, > { "808622C1", LPSS_ADDR(byt_i2c_dev_desc) }, > > - { "INT3430", LPSS_ADDR(lpt_dev_desc) }, > - { "INT3431", LPSS_ADDR(lpt_dev_desc) }, > - { "INT3432", LPSS_ADDR(lpt_i2c_dev_desc) }, > - { "INT3433", LPSS_ADDR(lpt_i2c_dev_desc) }, > - { "INT3434", LPSS_ADDR(lpt_uart_dev_desc) }, > - { "INT3435", LPSS_ADDR(lpt_uart_dev_desc) }, > - { "INT3436", LPSS_ADDR(lpt_sdio_dev_desc) }, > + { "INT3430", LPSS_ADDR(bsw_lpt_dev_desc) }, > + { "INT3431", LPSS_ADDR(bsw_lpt_dev_desc) }, > + { "INT3432", LPSS_ADDR(bsw_lpt_i2c_dev_desc) }, > + { "INT3433", LPSS_ADDR(bsw_lpt_i2c_dev_desc) }, > + { "INT3434", LPSS_ADDR(bsw_lpt_uart_dev_desc) }, > + { "INT3435", LPSS_ADDR(bsw_lpt_uart_dev_desc) }, > + { "INT3436", LPSS_ADDR(bsw_lpt_sdio_dev_desc) }, ... these are Broadwell devices. Have you tested that this won't break existing Broadwell systems? ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] ACPI / LPSS: Ignore 10ms delay for Braswell and Baytrail 2015-08-20 12:38 ` [PATCH] ACPI / LPSS: Ignore 10ms delay for Braswell and Baytrail Mika Westerberg @ 2015-08-20 13:04 ` Heikki Krogerus 2015-08-21 12:16 ` Kasagar, Srinidhi 2015-08-21 12:11 ` Kasagar, Srinidhi 1 sibling, 1 reply; 15+ messages in thread From: Heikki Krogerus @ 2015-08-20 13:04 UTC (permalink / raw) To: Mika Westerberg Cc: Srinidhi Kasagar, linux-acpi, rafael.j.wysocki, Kumar P Mahesh Hi, On Thu, Aug 20, 2015 at 03:38:05PM +0300, Mika Westerberg wrote: > +Heikki > > On Thu, Aug 20, 2015 at 10:46:07PM +0530, Srinidhi Kasagar wrote: > > LPSS devices in Braswell and Baytrail does not need the default > > 10ms d3_delay imposed by PCI specification. Removing this > > unnecessary delay significantly reduces the resume time > > (~200ms on Braswell/Cherrytrail) on these platforms. > > > > Signed-off-by: Srinidhi Kasagar <srinidhi.kasagar@intel.com> > > Signed-off-by: Kumar P Mahesh <mahesh.kumar.p@intel.com> > > Have you tested this on Asus T100? The delay was actually needed in > order to restore the context IIRC. We need to make sure the write operation succeeded when restoring the register values. That was the problem we had with T100, which btw. is Baytrail. Instead of using the delay conditionally, why not just read the value back in a loop (with timeout of course) until we see the write succeed? That should speedup the resume like you want, but still guarantee the ctx has really been restored. Thanks, -- heikki ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] ACPI / LPSS: Ignore 10ms delay for Braswell and Baytrail 2015-08-20 13:04 ` Heikki Krogerus @ 2015-08-21 12:16 ` Kasagar, Srinidhi 0 siblings, 0 replies; 15+ messages in thread From: Kasagar, Srinidhi @ 2015-08-21 12:16 UTC (permalink / raw) To: Heikki Krogerus Cc: Mika Westerberg, linux-acpi, rafael.j.wysocki, Kumar P Mahesh On Thu, Aug 20, 2015 at 04:04:47PM +0300, Heikki Krogerus wrote: > Hi, > > On Thu, Aug 20, 2015 at 03:38:05PM +0300, Mika Westerberg wrote: > > +Heikki > > > > On Thu, Aug 20, 2015 at 10:46:07PM +0530, Srinidhi Kasagar wrote: > > > LPSS devices in Braswell and Baytrail does not need the default > > > 10ms d3_delay imposed by PCI specification. Removing this > > > unnecessary delay significantly reduces the resume time > > > (~200ms on Braswell/Cherrytrail) on these platforms. > > > > > > Signed-off-by: Srinidhi Kasagar <srinidhi.kasagar@intel.com> > > > Signed-off-by: Kumar P Mahesh <mahesh.kumar.p@intel.com> > > > > Have you tested this on Asus T100? The delay was actually needed in > > order to restore the context IIRC. > > We need to make sure the write operation succeeded when restoring the > register values. That was the problem we had with T100, which btw. is > Baytrail. > > Instead of using the delay conditionally, why not just read the value > back in a loop (with timeout of course) until we see the write > succeed? That should speedup the resume like you want, but still > guarantee the ctx has really been restored. I would love to do that. But these are PCI devices and the delay is imposed by PCI spec and in many other places these conditional delays have been used. I do not think there exist any mechanism to verify write succeeds. Srinidhi ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] ACPI / LPSS: Ignore 10ms delay for Braswell and Baytrail 2015-08-20 12:38 ` [PATCH] ACPI / LPSS: Ignore 10ms delay for Braswell and Baytrail Mika Westerberg 2015-08-20 13:04 ` Heikki Krogerus @ 2015-08-21 12:11 ` Kasagar, Srinidhi 2015-08-21 6:36 ` Mika Westerberg 1 sibling, 1 reply; 15+ messages in thread From: Kasagar, Srinidhi @ 2015-08-21 12:11 UTC (permalink / raw) To: Mika Westerberg Cc: linux-acpi, rafael.j.wysocki, Kumar P Mahesh, Heikki Krogerus On Thu, Aug 20, 2015 at 03:38:05PM +0300, Mika Westerberg wrote: > +Heikki > > On Thu, Aug 20, 2015 at 10:46:07PM +0530, Srinidhi Kasagar wrote: > > LPSS devices in Braswell and Baytrail does not need the default > > 10ms d3_delay imposed by PCI specification. Removing this > > unnecessary delay significantly reduces the resume time > > (~200ms on Braswell/Cherrytrail) on these platforms. > > > > Signed-off-by: Srinidhi Kasagar <srinidhi.kasagar@intel.com> > > Signed-off-by: Kumar P Mahesh <mahesh.kumar.p@intel.com> > > Have you tested this on Asus T100? The delay was actually needed in > order to restore the context IIRC. Sorry, I do not have T100 h/w :( > > Also you are saying Braswell and Baytrail but... > > > --- > > drivers/acpi/acpi_lpss.c | 59 ++++++++++++++++++++++++++++++++++++---------- > > 1 file changed, 46 insertions(+), 13 deletions(-) > > [...] > > @@ -219,13 +247,13 @@ static const struct acpi_device_id acpi_lpss_device_ids[] = { > > { "8086228E", LPSS_ADDR(bsw_spi_dev_desc) }, > > { "808622C1", LPSS_ADDR(byt_i2c_dev_desc) }, > > > > - { "INT3430", LPSS_ADDR(lpt_dev_desc) }, > > - { "INT3431", LPSS_ADDR(lpt_dev_desc) }, > > - { "INT3432", LPSS_ADDR(lpt_i2c_dev_desc) }, > > - { "INT3433", LPSS_ADDR(lpt_i2c_dev_desc) }, > > - { "INT3434", LPSS_ADDR(lpt_uart_dev_desc) }, > > - { "INT3435", LPSS_ADDR(lpt_uart_dev_desc) }, > > - { "INT3436", LPSS_ADDR(lpt_sdio_dev_desc) }, > > + { "INT3430", LPSS_ADDR(bsw_lpt_dev_desc) }, > > + { "INT3431", LPSS_ADDR(bsw_lpt_dev_desc) }, > > + { "INT3432", LPSS_ADDR(bsw_lpt_i2c_dev_desc) }, > > + { "INT3433", LPSS_ADDR(bsw_lpt_i2c_dev_desc) }, > > + { "INT3434", LPSS_ADDR(bsw_lpt_uart_dev_desc) }, > > + { "INT3435", LPSS_ADDR(bsw_lpt_uart_dev_desc) }, > > + { "INT3436", LPSS_ADDR(bsw_lpt_sdio_dev_desc) }, > > ... these are Broadwell devices. Have you tested that this won't break > existing Broadwell systems? You mean these are Broadwell devices too? As of now I have tested these devices to be working on Braswell and Cherrytrail. Srinidhi ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] ACPI / LPSS: Ignore 10ms delay for Braswell and Baytrail 2015-08-21 12:11 ` Kasagar, Srinidhi @ 2015-08-21 6:36 ` Mika Westerberg 2015-08-21 13:20 ` Mika Westerberg 0 siblings, 1 reply; 15+ messages in thread From: Mika Westerberg @ 2015-08-21 6:36 UTC (permalink / raw) To: Kasagar, Srinidhi Cc: linux-acpi, rafael.j.wysocki, Kumar P Mahesh, Heikki Krogerus On Fri, Aug 21, 2015 at 05:41:52PM +0530, Kasagar, Srinidhi wrote: > On Thu, Aug 20, 2015 at 03:38:05PM +0300, Mika Westerberg wrote: > > +Heikki > > > > On Thu, Aug 20, 2015 at 10:46:07PM +0530, Srinidhi Kasagar wrote: > > > LPSS devices in Braswell and Baytrail does not need the default > > > 10ms d3_delay imposed by PCI specification. Removing this > > > unnecessary delay significantly reduces the resume time > > > (~200ms on Braswell/Cherrytrail) on these platforms. > > > > > > Signed-off-by: Srinidhi Kasagar <srinidhi.kasagar@intel.com> > > > Signed-off-by: Kumar P Mahesh <mahesh.kumar.p@intel.com> > > > > Have you tested this on Asus T100? The delay was actually needed in > > order to restore the context IIRC. > > Sorry, I do not have T100 h/w :( OK. We have one and I'm going to test this patch on it. In particular the T100 needed to have these delays otherwise writes failed. > > > > Also you are saying Braswell and Baytrail but... > > > > > --- > > > drivers/acpi/acpi_lpss.c | 59 ++++++++++++++++++++++++++++++++++++---------- > > > 1 file changed, 46 insertions(+), 13 deletions(-) > > > > > [...] > > > > @@ -219,13 +247,13 @@ static const struct acpi_device_id acpi_lpss_device_ids[] = { > > > { "8086228E", LPSS_ADDR(bsw_spi_dev_desc) }, > > > { "808622C1", LPSS_ADDR(byt_i2c_dev_desc) }, > > > > > > - { "INT3430", LPSS_ADDR(lpt_dev_desc) }, > > > - { "INT3431", LPSS_ADDR(lpt_dev_desc) }, > > > - { "INT3432", LPSS_ADDR(lpt_i2c_dev_desc) }, > > > - { "INT3433", LPSS_ADDR(lpt_i2c_dev_desc) }, > > > - { "INT3434", LPSS_ADDR(lpt_uart_dev_desc) }, > > > - { "INT3435", LPSS_ADDR(lpt_uart_dev_desc) }, > > > - { "INT3436", LPSS_ADDR(lpt_sdio_dev_desc) }, > > > + { "INT3430", LPSS_ADDR(bsw_lpt_dev_desc) }, > > > + { "INT3431", LPSS_ADDR(bsw_lpt_dev_desc) }, > > > + { "INT3432", LPSS_ADDR(bsw_lpt_i2c_dev_desc) }, > > > + { "INT3433", LPSS_ADDR(bsw_lpt_i2c_dev_desc) }, > > > + { "INT3434", LPSS_ADDR(bsw_lpt_uart_dev_desc) }, > > > + { "INT3435", LPSS_ADDR(bsw_lpt_uart_dev_desc) }, > > > + { "INT3436", LPSS_ADDR(bsw_lpt_sdio_dev_desc) }, > > > > ... these are Broadwell devices. Have you tested that this won't break > > existing Broadwell systems? > > You mean these are Broadwell devices too? As of now I have > tested these devices to be working on Braswell and Cherrytrail. Yes, the ACPI IDs you are changing above are Broadwell ACPI IDs (INT34xx). ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] ACPI / LPSS: Ignore 10ms delay for Braswell and Baytrail 2015-08-21 6:36 ` Mika Westerberg @ 2015-08-21 13:20 ` Mika Westerberg 2015-08-24 12:51 ` Kasagar, Srinidhi 0 siblings, 1 reply; 15+ messages in thread From: Mika Westerberg @ 2015-08-21 13:20 UTC (permalink / raw) To: Kasagar, Srinidhi Cc: linux-acpi, rafael.j.wysocki, Kumar P Mahesh, Heikki Krogerus On Fri, Aug 21, 2015 at 09:36:41AM +0300, Mika Westerberg wrote: > On Fri, Aug 21, 2015 at 05:41:52PM +0530, Kasagar, Srinidhi wrote: > > On Thu, Aug 20, 2015 at 03:38:05PM +0300, Mika Westerberg wrote: > > > +Heikki > > > > > > On Thu, Aug 20, 2015 at 10:46:07PM +0530, Srinidhi Kasagar wrote: > > > > LPSS devices in Braswell and Baytrail does not need the default > > > > 10ms d3_delay imposed by PCI specification. Removing this > > > > unnecessary delay significantly reduces the resume time > > > > (~200ms on Braswell/Cherrytrail) on these platforms. > > > > > > > > Signed-off-by: Srinidhi Kasagar <srinidhi.kasagar@intel.com> > > > > Signed-off-by: Kumar P Mahesh <mahesh.kumar.p@intel.com> > > > > > > Have you tested this on Asus T100? The delay was actually needed in > > > order to restore the context IIRC. > > > > Sorry, I do not have T100 h/w :( > > OK. We have one and I'm going to test this patch on it. In particular > the T100 needed to have these delays otherwise writes failed. The patch does not apply on top of v4.2-rc7 or linux-pm/bleeding-edge. So I just unconditionally set the delay to 0 for all LPSS devices on T100. Having msleep(0) seems to be enough and the touch panel including I2C host controller runtime resumes just fine :-) If I remove msleep() completely then things break apart and runtime resuming the touch panel triggers this: [ 46.143111] i2c_hid i2c-ATML1000:00: i2c_hid_set_power [ 46.145758] i2c_hid i2c-ATML1000:00: __i2c_hid_command: cmd=fb 00 00 08 [ 46.148483] i2c_designware 80860F41:05: Unknown Synopsys component type: 0x00000000 [ 46.252125] i2c_designware 80860F41:05: timeout in enabling adapter [ 47.254426] i2c_designware 80860F41:05: controller timed out So clearly it needs some delay but it does not have to be 10ms. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] ACPI / LPSS: Ignore 10ms delay for Braswell and Baytrail 2015-08-21 13:20 ` Mika Westerberg @ 2015-08-24 12:51 ` Kasagar, Srinidhi 2015-08-24 8:59 ` Mika Westerberg 0 siblings, 1 reply; 15+ messages in thread From: Kasagar, Srinidhi @ 2015-08-24 12:51 UTC (permalink / raw) To: Mika Westerberg Cc: linux-acpi, rafael.j.wysocki, Kumar P Mahesh, Heikki Krogerus On Fri, Aug 21, 2015 at 04:20:15PM +0300, Mika Westerberg wrote: > On Fri, Aug 21, 2015 at 09:36:41AM +0300, Mika Westerberg wrote: > > On Fri, Aug 21, 2015 at 05:41:52PM +0530, Kasagar, Srinidhi wrote: > > > On Thu, Aug 20, 2015 at 03:38:05PM +0300, Mika Westerberg wrote: > > > > +Heikki > > > > > > > > On Thu, Aug 20, 2015 at 10:46:07PM +0530, Srinidhi Kasagar wrote: > > > > > LPSS devices in Braswell and Baytrail does not need the default > > > > > 10ms d3_delay imposed by PCI specification. Removing this > > > > > unnecessary delay significantly reduces the resume time > > > > > (~200ms on Braswell/Cherrytrail) on these platforms. > > > > > > > > > > Signed-off-by: Srinidhi Kasagar <srinidhi.kasagar@intel.com> > > > > > Signed-off-by: Kumar P Mahesh <mahesh.kumar.p@intel.com> > > > > > > > > Have you tested this on Asus T100? The delay was actually needed in > > > > order to restore the context IIRC. > > > > > > Sorry, I do not have T100 h/w :( > > > > OK. We have one and I'm going to test this patch on it. In particular > > the T100 needed to have these delays otherwise writes failed. > > The patch does not apply on top of v4.2-rc7 or linux-pm/bleeding-edge. > So I just unconditionally set the delay to 0 for all LPSS devices on > T100. > > Having msleep(0) seems to be enough and the touch panel including I2C > host controller runtime resumes just fine :-) > > If I remove msleep() completely then things break apart and runtime > resuming the touch panel triggers this: > > [ 46.143111] i2c_hid i2c-ATML1000:00: i2c_hid_set_power > [ 46.145758] i2c_hid i2c-ATML1000:00: __i2c_hid_command: cmd=fb 00 00 08 > [ 46.148483] i2c_designware 80860F41:05: Unknown Synopsys component type: 0x00000000 > [ 46.252125] i2c_designware 80860F41:05: timeout in enabling adapter > [ 47.254426] i2c_designware 80860F41:05: controller timed out > > So clearly it needs some delay but it does not have to be 10ms. Hm..Some internal south devices in Atom platforms does need ~3ms d3_delay which we have fixed them as PCI fixups. So, do you think it make sense to selectively pick those kind of devices as fixups and remove the conditional delay entirely from this part? Apart from touch/i2c what else breaks in T100? Srinidhi ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] ACPI / LPSS: Ignore 10ms delay for Braswell and Baytrail 2015-08-24 12:51 ` Kasagar, Srinidhi @ 2015-08-24 8:59 ` Mika Westerberg 2015-08-24 17:09 ` Kasagar, Srinidhi 0 siblings, 1 reply; 15+ messages in thread From: Mika Westerberg @ 2015-08-24 8:59 UTC (permalink / raw) To: Kasagar, Srinidhi Cc: linux-acpi, rafael.j.wysocki, Kumar P Mahesh, Heikki Krogerus On Mon, Aug 24, 2015 at 06:21:47PM +0530, Kasagar, Srinidhi wrote: > On Fri, Aug 21, 2015 at 04:20:15PM +0300, Mika Westerberg wrote: > > On Fri, Aug 21, 2015 at 09:36:41AM +0300, Mika Westerberg wrote: > > > On Fri, Aug 21, 2015 at 05:41:52PM +0530, Kasagar, Srinidhi wrote: > > > > On Thu, Aug 20, 2015 at 03:38:05PM +0300, Mika Westerberg wrote: > > > > > +Heikki > > > > > > > > > > On Thu, Aug 20, 2015 at 10:46:07PM +0530, Srinidhi Kasagar wrote: > > > > > > LPSS devices in Braswell and Baytrail does not need the default > > > > > > 10ms d3_delay imposed by PCI specification. Removing this > > > > > > unnecessary delay significantly reduces the resume time > > > > > > (~200ms on Braswell/Cherrytrail) on these platforms. > > > > > > > > > > > > Signed-off-by: Srinidhi Kasagar <srinidhi.kasagar@intel.com> > > > > > > Signed-off-by: Kumar P Mahesh <mahesh.kumar.p@intel.com> > > > > > > > > > > Have you tested this on Asus T100? The delay was actually needed in > > > > > order to restore the context IIRC. > > > > > > > > Sorry, I do not have T100 h/w :( > > > > > > OK. We have one and I'm going to test this patch on it. In particular > > > the T100 needed to have these delays otherwise writes failed. > > > > The patch does not apply on top of v4.2-rc7 or linux-pm/bleeding-edge. > > So I just unconditionally set the delay to 0 for all LPSS devices on > > T100. > > > > Having msleep(0) seems to be enough and the touch panel including I2C > > host controller runtime resumes just fine :-) > > > > If I remove msleep() completely then things break apart and runtime > > resuming the touch panel triggers this: > > > > [ 46.143111] i2c_hid i2c-ATML1000:00: i2c_hid_set_power > > [ 46.145758] i2c_hid i2c-ATML1000:00: __i2c_hid_command: cmd=fb 00 00 08 > > [ 46.148483] i2c_designware 80860F41:05: Unknown Synopsys component type: 0x00000000 > > [ 46.252125] i2c_designware 80860F41:05: timeout in enabling adapter > > [ 47.254426] i2c_designware 80860F41:05: controller timed out > > > > So clearly it needs some delay but it does not have to be 10ms. > > Hm..Some internal south devices in Atom platforms does need ~3ms d3_delay > which we have fixed them as PCI fixups. So, do you think it make sense > to selectively pick those kind of devices as fixups and remove the > conditional delay entirely from this part? Depends. If we need to add more and more devices to that fixup list, I don't think it makes sense. > Apart from touch/i2c what else breaks in T100? I suppose HS-UART and SPI both break as well because the interface clock is disabled after reset. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] ACPI / LPSS: Ignore 10ms delay for Braswell and Baytrail 2015-08-24 8:59 ` Mika Westerberg @ 2015-08-24 17:09 ` Kasagar, Srinidhi 2015-08-24 9:44 ` Mika Westerberg 0 siblings, 1 reply; 15+ messages in thread From: Kasagar, Srinidhi @ 2015-08-24 17:09 UTC (permalink / raw) To: Mika Westerberg Cc: linux-acpi, rafael.j.wysocki, Kumar P Mahesh, Heikki Krogerus On Mon, Aug 24, 2015 at 11:59:59AM +0300, Mika Westerberg wrote: > On Mon, Aug 24, 2015 at 06:21:47PM +0530, Kasagar, Srinidhi wrote: > > On Fri, Aug 21, 2015 at 04:20:15PM +0300, Mika Westerberg wrote: > > > On Fri, Aug 21, 2015 at 09:36:41AM +0300, Mika Westerberg wrote: > > > > On Fri, Aug 21, 2015 at 05:41:52PM +0530, Kasagar, Srinidhi wrote: > > > > > On Thu, Aug 20, 2015 at 03:38:05PM +0300, Mika Westerberg wrote: > > > > > > +Heikki > > > > > > > > > > > > On Thu, Aug 20, 2015 at 10:46:07PM +0530, Srinidhi Kasagar wrote: > > > > > > > LPSS devices in Braswell and Baytrail does not need the default > > > > > > > 10ms d3_delay imposed by PCI specification. Removing this > > > > > > > unnecessary delay significantly reduces the resume time > > > > > > > (~200ms on Braswell/Cherrytrail) on these platforms. > > > > > > > > > > > > > > Signed-off-by: Srinidhi Kasagar <srinidhi.kasagar@intel.com> > > > > > > > Signed-off-by: Kumar P Mahesh <mahesh.kumar.p@intel.com> > > > > > > > > > > > > Have you tested this on Asus T100? The delay was actually needed in > > > > > > order to restore the context IIRC. > > > > > > > > > > Sorry, I do not have T100 h/w :( > > > > > > > > OK. We have one and I'm going to test this patch on it. In particular > > > > the T100 needed to have these delays otherwise writes failed. > > > > > > The patch does not apply on top of v4.2-rc7 or linux-pm/bleeding-edge. > > > So I just unconditionally set the delay to 0 for all LPSS devices on > > > T100. > > > > > > Having msleep(0) seems to be enough and the touch panel including I2C > > > host controller runtime resumes just fine :-) > > > > > > If I remove msleep() completely then things break apart and runtime > > > resuming the touch panel triggers this: > > > > > > [ 46.143111] i2c_hid i2c-ATML1000:00: i2c_hid_set_power > > > [ 46.145758] i2c_hid i2c-ATML1000:00: __i2c_hid_command: cmd=fb 00 00 08 > > > [ 46.148483] i2c_designware 80860F41:05: Unknown Synopsys component type: 0x00000000 > > > [ 46.252125] i2c_designware 80860F41:05: timeout in enabling adapter > > > [ 47.254426] i2c_designware 80860F41:05: controller timed out > > > > > > So clearly it needs some delay but it does not have to be 10ms. > > > > Hm..Some internal south devices in Atom platforms does need ~3ms d3_delay > > which we have fixed them as PCI fixups. So, do you think it make sense > > to selectively pick those kind of devices as fixups and remove the > > conditional delay entirely from this part? > > Depends. If we need to add more and more devices to that fixup list, I > don't think it makes sense. Hmm..I see a lot of such existing stuffs and keeps coming.. > > > Apart from touch/i2c what else breaks in T100? > > I suppose HS-UART and SPI both break as well because the interface clock > is disabled after reset. Ok, then we have two options: Let's drop LPSS_NO_D3_DELAY for BYT and duplicate the common devices which are shared between Braswell and BYT & keep LPSS_NO_D3_DELAY only for BSW. OR Use the cpu id to detect the platform which I believe not a good idea.. Because of few devices, I do not think it make sense to compromise on the significant reduction in the resume delay. Srinidhi ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] ACPI / LPSS: Ignore 10ms delay for Braswell and Baytrail 2015-08-24 17:09 ` Kasagar, Srinidhi @ 2015-08-24 9:44 ` Mika Westerberg 2015-08-27 14:39 ` Kasagar, Srinidhi 0 siblings, 1 reply; 15+ messages in thread From: Mika Westerberg @ 2015-08-24 9:44 UTC (permalink / raw) To: Kasagar, Srinidhi Cc: linux-acpi, rafael.j.wysocki, Kumar P Mahesh, Heikki Krogerus On Mon, Aug 24, 2015 at 10:39:21PM +0530, Kasagar, Srinidhi wrote: > Ok, then we have two options: > > Let's drop LPSS_NO_D3_DELAY for BYT and duplicate the common devices which > are shared between Braswell and BYT & keep LPSS_NO_D3_DELAY only for BSW. That sounds the safest option for now. > OR > Use the cpu id to detect the platform which I believe not a good idea.. > > Because of few devices, I do not think it make sense to compromise on > the significant reduction in the resume delay. I agree. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] ACPI / LPSS: Ignore 10ms delay for Braswell and Baytrail 2015-08-24 9:44 ` Mika Westerberg @ 2015-08-27 14:39 ` Kasagar, Srinidhi 2015-08-27 7:14 ` Mika Westerberg 0 siblings, 1 reply; 15+ messages in thread From: Kasagar, Srinidhi @ 2015-08-27 14:39 UTC (permalink / raw) To: Mika Westerberg Cc: linux-acpi, rafael.j.wysocki, Kumar P Mahesh, Heikki Krogerus On Mon, Aug 24, 2015 at 12:44:55PM +0300, Mika Westerberg wrote: > On Mon, Aug 24, 2015 at 10:39:21PM +0530, Kasagar, Srinidhi wrote: > > Ok, then we have two options: > > > > Let's drop LPSS_NO_D3_DELAY for BYT and duplicate the common devices which > > are shared between Braswell and BYT & keep LPSS_NO_D3_DELAY only for BSW. > > That sounds the safest option for now. > > > OR > > Use the cpu id to detect the platform which I believe not a good idea.. > > > > Because of few devices, I do not think it make sense to compromise on > > the significant reduction in the resume delay. > > I agree. Refreshed the patch (below) affecting BSW platform alone. But this does not solve your earlier concern on Broadwell which shares the same acpi ids.. >From aa6861fee9e9ea4449c3c2da9292b1af1382d38f Mon Sep 17 00:00:00 2001 From: Srinidhi Kasagar <srinidhi.kasagar@intel.com> Date: Thu, 27 Aug 2015 19:58:32 +0530 Subject: [PATCH] ACPI / LPSS: Ignore 10ms delay for Braswell LPSS devices in Braswell does not need the default 10ms d3_delay imposed by PCI specification. Removing this unnecessary delay significantly reduces the resume time approximately upto 200ms on this platform. Signed-off-by: Srinidhi Kasagar <srinidhi.kasagar@intel.com> --- drivers/acpi/acpi_lpss.c | 78 +++++++++++++++++++++++++++++++++++++--------- 1 file changed, 64 insertions(+), 14 deletions(-) diff --git a/drivers/acpi/acpi_lpss.c b/drivers/acpi/acpi_lpss.c index 46b58abb08c5..d3361d6e2e6e 100644 --- a/drivers/acpi/acpi_lpss.c +++ b/drivers/acpi/acpi_lpss.c @@ -60,6 +60,7 @@ ACPI_MODULE_NAME("acpi_lpss"); #define LPSS_CLK_DIVIDER BIT(2) #define LPSS_LTR BIT(3) #define LPSS_SAVE_CTX BIT(4) +#define LPSS_NO_D3_DELAY BIT(5) struct lpss_private_data; @@ -134,11 +135,22 @@ static const struct lpss_device_desc lpt_dev_desc = { .prv_offset = 0x800, }; +static const struct lpss_device_desc bsw_lpt_dev_desc = { + .flags = LPSS_CLK | LPSS_CLK_GATE | LPSS_CLK_DIVIDER | LPSS_LTR + | LPSS_NO_D3_DELAY, + .prv_offset = 0x800, +}; + static const struct lpss_device_desc lpt_i2c_dev_desc = { .flags = LPSS_CLK | LPSS_CLK_GATE | LPSS_LTR, .prv_offset = 0x800, }; +static const struct lpss_device_desc bsw_lpt_i2c_dev_desc = { + .flags = LPSS_CLK | LPSS_CLK_GATE | LPSS_LTR | LPSS_NO_D3_DELAY, + .prv_offset = 0x800, +}; + static const struct lpss_device_desc lpt_uart_dev_desc = { .flags = LPSS_CLK | LPSS_CLK_GATE | LPSS_CLK_DIVIDER | LPSS_LTR, .clk_con_id = "baudclk", @@ -146,16 +158,34 @@ static const struct lpss_device_desc lpt_uart_dev_desc = { .setup = lpss_uart_setup, }; +static const struct lpss_device_desc bsw_lpt_uart_dev_desc = { + .flags = LPSS_CLK | LPSS_CLK_GATE | LPSS_CLK_DIVIDER | LPSS_LTR + | LPSS_NO_D3_DELAY, + .clk_con_id = "baudclk", + .prv_offset = 0x800, + .setup = lpss_uart_setup, +}; + static const struct lpss_device_desc lpt_sdio_dev_desc = { .flags = LPSS_LTR, .prv_offset = 0x1000, .prv_size_override = 0x1018, }; +static const struct lpss_device_desc bsw_lpt_sdio_dev_desc = { + .flags = LPSS_LTR | LPSS_NO_D3_DELAY, + .prv_offset = 0x1000, + .prv_size_override = 0x1018, +}; + static const struct lpss_device_desc byt_pwm_dev_desc = { .flags = LPSS_SAVE_CTX, }; +static const struct lpss_device_desc bsw_pwm_dev_desc = { + .flags = LPSS_SAVE_CTX | LPSS_NO_D3_DELAY, +}; + static const struct lpss_device_desc byt_uart_dev_desc = { .flags = LPSS_CLK | LPSS_CLK_GATE | LPSS_CLK_DIVIDER | LPSS_SAVE_CTX, .clk_con_id = "baudclk", @@ -178,8 +208,23 @@ static const struct lpss_device_desc byt_i2c_dev_desc = { .setup = byt_i2c_setup, }; +static const struct lpss_device_desc bsw_i2c_dev_desc = { + .flags = LPSS_CLK | LPSS_SAVE_CTX | LPSS_NO_D3_DELAY, + .prv_offset = 0x800, + .setup = byt_i2c_setup, +}; + +static const struct lpss_device_desc bsw_uart_dev_desc = { + .flags = LPSS_CLK | LPSS_CLK_GATE | LPSS_CLK_DIVIDER | LPSS_SAVE_CTX + | LPSS_NO_D3_DELAY, + .clk_con_id = "baudclk", + .prv_offset = 0x800, + .setup = lpss_uart_setup, +}; + static struct lpss_device_desc bsw_spi_dev_desc = { - .flags = LPSS_CLK | LPSS_CLK_GATE | LPSS_CLK_DIVIDER | LPSS_SAVE_CTX, + .flags = LPSS_CLK | LPSS_CLK_GATE | LPSS_CLK_DIVIDER | LPSS_SAVE_CTX + | LPSS_NO_D3_DELAY, .prv_offset = 0x400, .setup = lpss_deassert_reset, }; @@ -214,18 +259,18 @@ static const struct acpi_device_id acpi_lpss_device_ids[] = { { "INT33FC", }, /* Braswell LPSS devices */ - { "80862288", LPSS_ADDR(byt_pwm_dev_desc) }, - { "8086228A", LPSS_ADDR(byt_uart_dev_desc) }, + { "80862288", LPSS_ADDR(bsw_pwm_dev_desc) }, + { "8086228A", LPSS_ADDR(bsw_uart_dev_desc) }, { "8086228E", LPSS_ADDR(bsw_spi_dev_desc) }, - { "808622C1", LPSS_ADDR(byt_i2c_dev_desc) }, - - { "INT3430", LPSS_ADDR(lpt_dev_desc) }, - { "INT3431", LPSS_ADDR(lpt_dev_desc) }, - { "INT3432", LPSS_ADDR(lpt_i2c_dev_desc) }, - { "INT3433", LPSS_ADDR(lpt_i2c_dev_desc) }, - { "INT3434", LPSS_ADDR(lpt_uart_dev_desc) }, - { "INT3435", LPSS_ADDR(lpt_uart_dev_desc) }, - { "INT3436", LPSS_ADDR(lpt_sdio_dev_desc) }, + { "808622C1", LPSS_ADDR(bsw_i2c_dev_desc) }, + + { "INT3430", LPSS_ADDR(bsw_lpt_dev_desc) }, + { "INT3431", LPSS_ADDR(bsw_lpt_dev_desc) }, + { "INT3432", LPSS_ADDR(bsw_lpt_i2c_dev_desc) }, + { "INT3433", LPSS_ADDR(bsw_lpt_i2c_dev_desc) }, + { "INT3434", LPSS_ADDR(bsw_lpt_uart_dev_desc) }, + { "INT3435", LPSS_ADDR(bsw_lpt_uart_dev_desc) }, + { "INT3436", LPSS_ADDR(bsw_lpt_sdio_dev_desc) }, { "INT3437", }, /* Wildcat Point LPSS devices */ @@ -558,9 +603,14 @@ static void acpi_lpss_restore_ctx(struct device *dev, * The following delay is needed or the subsequent write operations may * fail. The LPSS devices are actually PCI devices and the PCI spec * expects 10ms delay before the device can be accessed after D3 to D0 - * transition. + * transition. However some platforms like BSW does not need this delay. */ - msleep(10); + unsigned int delay = 10; /* default 10ms delay */ + + if (pdata->dev_desc->flags & LPSS_NO_D3_DELAY) + delay = 0; + + msleep(delay); for (i = 0; i < LPSS_PRV_REG_COUNT; i++) { unsigned long offset = i * sizeof(u32); -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH] ACPI / LPSS: Ignore 10ms delay for Braswell and Baytrail 2015-08-27 14:39 ` Kasagar, Srinidhi @ 2015-08-27 7:14 ` Mika Westerberg 2015-08-27 16:13 ` Kasagar, Srinidhi 0 siblings, 1 reply; 15+ messages in thread From: Mika Westerberg @ 2015-08-27 7:14 UTC (permalink / raw) To: Kasagar, Srinidhi Cc: linux-acpi, rafael.j.wysocki, Kumar P Mahesh, Heikki Krogerus On Thu, Aug 27, 2015 at 08:09:55PM +0530, Kasagar, Srinidhi wrote: > On Mon, Aug 24, 2015 at 12:44:55PM +0300, Mika Westerberg wrote: > > On Mon, Aug 24, 2015 at 10:39:21PM +0530, Kasagar, Srinidhi wrote: > > > Ok, then we have two options: > > > > > > Let's drop LPSS_NO_D3_DELAY for BYT and duplicate the common devices which > > > are shared between Braswell and BYT & keep LPSS_NO_D3_DELAY only for BSW. > > > > That sounds the safest option for now. > > > > > OR > > > Use the cpu id to detect the platform which I believe not a good idea.. > > > > > > Because of few devices, I do not think it make sense to compromise on > > > the significant reduction in the resume delay. > > > > I agree. > > Refreshed the patch (below) affecting BSW platform alone. > > But this does not solve your earlier concern on Broadwell which > shares the same acpi ids.. It does not share IDs: /* Braswell LPSS devices */ { "80862288", LPSS_ADDR(byt_pwm_dev_desc) }, { "8086228A", LPSS_ADDR(byt_uart_dev_desc) }, { "8086228E", LPSS_ADDR(bsw_spi_dev_desc) }, { "808622C1", LPSS_ADDR(byt_i2c_dev_desc) }, /* The following are Broadwell LPSS IDs _not_ Braswell */ { "INT3430", LPSS_ADDR(lpt_dev_desc) }, { "INT3431", LPSS_ADDR(lpt_dev_desc) }, { "INT3432", LPSS_ADDR(lpt_i2c_dev_desc) }, { "INT3433", LPSS_ADDR(lpt_i2c_dev_desc) }, { "INT3434", LPSS_ADDR(lpt_uart_dev_desc) }, { "INT3435", LPSS_ADDR(lpt_uart_dev_desc) }, { "INT3436", LPSS_ADDR(lpt_sdio_dev_desc) }, { "INT3437", }, So please make the patch touch only the four 808622* devices above unless you have tested this on Broadwell as well. Otherwise the patch looks good to me. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] ACPI / LPSS: Ignore 10ms delay for Braswell and Baytrail 2015-08-27 7:14 ` Mika Westerberg @ 2015-08-27 16:13 ` Kasagar, Srinidhi 2015-08-27 8:30 ` Mika Westerberg 0 siblings, 1 reply; 15+ messages in thread From: Kasagar, Srinidhi @ 2015-08-27 16:13 UTC (permalink / raw) To: Mika Westerberg Cc: linux-acpi, rafael.j.wysocki, Kumar P Mahesh, Heikki Krogerus On Thu, Aug 27, 2015 at 10:14:14AM +0300, Mika Westerberg wrote: > On Thu, Aug 27, 2015 at 08:09:55PM +0530, Kasagar, Srinidhi wrote: > > On Mon, Aug 24, 2015 at 12:44:55PM +0300, Mika Westerberg wrote: > > > On Mon, Aug 24, 2015 at 10:39:21PM +0530, Kasagar, Srinidhi wrote: > > > > Ok, then we have two options: > > > > > > > > Let's drop LPSS_NO_D3_DELAY for BYT and duplicate the common devices which > > > > are shared between Braswell and BYT & keep LPSS_NO_D3_DELAY only for BSW. > > > > > > That sounds the safest option for now. > > > > > > > OR > > > > Use the cpu id to detect the platform which I believe not a good idea.. > > > > > > > > Because of few devices, I do not think it make sense to compromise on > > > > the significant reduction in the resume delay. > > > > > > I agree. > > > > Refreshed the patch (below) affecting BSW platform alone. > > > > But this does not solve your earlier concern on Broadwell which > > shares the same acpi ids.. > > It does not share IDs: Oh, Thanks.. > > /* Braswell LPSS devices */ > { "80862288", LPSS_ADDR(byt_pwm_dev_desc) }, > { "8086228A", LPSS_ADDR(byt_uart_dev_desc) }, > { "8086228E", LPSS_ADDR(bsw_spi_dev_desc) }, > { "808622C1", LPSS_ADDR(byt_i2c_dev_desc) }, > > /* The following are Broadwell LPSS IDs _not_ Braswell */ > { "INT3430", LPSS_ADDR(lpt_dev_desc) }, > { "INT3431", LPSS_ADDR(lpt_dev_desc) }, > { "INT3432", LPSS_ADDR(lpt_i2c_dev_desc) }, > { "INT3433", LPSS_ADDR(lpt_i2c_dev_desc) }, > { "INT3434", LPSS_ADDR(lpt_uart_dev_desc) }, > { "INT3435", LPSS_ADDR(lpt_uart_dev_desc) }, > { "INT3436", LPSS_ADDR(lpt_sdio_dev_desc) }, > { "INT3437", }, > > So please make the patch touch only the four 808622* devices above > unless you have tested this on Broadwell as well. > > Otherwise the patch looks good to me. Here is the patch.. >From 970c704dbb05aa8392b6e1090ceaf9ed76b6436b Mon Sep 17 00:00:00 2001 From: Srinidhi Kasagar <srinidhi.kasagar@intel.com> Date: Thu, 27 Aug 2015 21:30:55 +0530 Subject: [PATCH] ACPI / LPSS: Ignore 10ms delay for Braswell LPSS devices in Braswell does not need the default 10ms d3_delay imposed by PCI specification. Removing this unnecessary delay significantly reduces the resume time approximately upto 200ms on this platform. Signed-off-by: Srinidhi Kasagar <srinidhi.kasagar@intel.com> --- drivers/acpi/acpi_lpss.c | 38 ++++++++++++++++++++++++++++++++------ 1 file changed, 32 insertions(+), 6 deletions(-) diff --git a/drivers/acpi/acpi_lpss.c b/drivers/acpi/acpi_lpss.c index 46b58abb08c5..10020e04f574 100644 --- a/drivers/acpi/acpi_lpss.c +++ b/drivers/acpi/acpi_lpss.c @@ -60,6 +60,7 @@ ACPI_MODULE_NAME("acpi_lpss"); #define LPSS_CLK_DIVIDER BIT(2) #define LPSS_LTR BIT(3) #define LPSS_SAVE_CTX BIT(4) +#define LPSS_NO_D3_DELAY BIT(5) struct lpss_private_data; @@ -156,6 +157,10 @@ static const struct lpss_device_desc byt_pwm_dev_desc = { .flags = LPSS_SAVE_CTX, }; +static const struct lpss_device_desc bsw_pwm_dev_desc = { + .flags = LPSS_SAVE_CTX | LPSS_NO_D3_DELAY, +}; + static const struct lpss_device_desc byt_uart_dev_desc = { .flags = LPSS_CLK | LPSS_CLK_GATE | LPSS_CLK_DIVIDER | LPSS_SAVE_CTX, .clk_con_id = "baudclk", @@ -163,6 +168,14 @@ static const struct lpss_device_desc byt_uart_dev_desc = { .setup = lpss_uart_setup, }; +static const struct lpss_device_desc bsw_uart_dev_desc = { + .flags = LPSS_CLK | LPSS_CLK_GATE | LPSS_CLK_DIVIDER | LPSS_SAVE_CTX + | LPSS_NO_D3_DELAY, + .clk_con_id = "baudclk", + .prv_offset = 0x800, + .setup = lpss_uart_setup, +}; + static const struct lpss_device_desc byt_spi_dev_desc = { .flags = LPSS_CLK | LPSS_CLK_GATE | LPSS_CLK_DIVIDER | LPSS_SAVE_CTX, .prv_offset = 0x400, @@ -178,8 +191,15 @@ static const struct lpss_device_desc byt_i2c_dev_desc = { .setup = byt_i2c_setup, }; +static const struct lpss_device_desc bsw_i2c_dev_desc = { + .flags = LPSS_CLK | LPSS_SAVE_CTX | LPSS_NO_D3_DELAY, + .prv_offset = 0x800, + .setup = byt_i2c_setup, +}; + static struct lpss_device_desc bsw_spi_dev_desc = { - .flags = LPSS_CLK | LPSS_CLK_GATE | LPSS_CLK_DIVIDER | LPSS_SAVE_CTX, + .flags = LPSS_CLK | LPSS_CLK_GATE | LPSS_CLK_DIVIDER | LPSS_SAVE_CTX + | LPSS_NO_D3_DELAY, .prv_offset = 0x400, .setup = lpss_deassert_reset, }; @@ -214,11 +234,12 @@ static const struct acpi_device_id acpi_lpss_device_ids[] = { { "INT33FC", }, /* Braswell LPSS devices */ - { "80862288", LPSS_ADDR(byt_pwm_dev_desc) }, - { "8086228A", LPSS_ADDR(byt_uart_dev_desc) }, + { "80862288", LPSS_ADDR(bsw_pwm_dev_desc) }, + { "8086228A", LPSS_ADDR(bsw_uart_dev_desc) }, { "8086228E", LPSS_ADDR(bsw_spi_dev_desc) }, - { "808622C1", LPSS_ADDR(byt_i2c_dev_desc) }, + { "808622C1", LPSS_ADDR(bsw_i2c_dev_desc) }, + /* Broadwell LPSS devices */ { "INT3430", LPSS_ADDR(lpt_dev_desc) }, { "INT3431", LPSS_ADDR(lpt_dev_desc) }, { "INT3432", LPSS_ADDR(lpt_i2c_dev_desc) }, @@ -558,9 +579,14 @@ static void acpi_lpss_restore_ctx(struct device *dev, * The following delay is needed or the subsequent write operations may * fail. The LPSS devices are actually PCI devices and the PCI spec * expects 10ms delay before the device can be accessed after D3 to D0 - * transition. + * transition. However some platforms like BSW does not need this delay. */ - msleep(10); + unsigned int delay = 10; /* default 10ms delay */ + + if (pdata->dev_desc->flags & LPSS_NO_D3_DELAY) + delay = 0; + + msleep(delay); for (i = 0; i < LPSS_PRV_REG_COUNT; i++) { unsigned long offset = i * sizeof(u32); -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH] ACPI / LPSS: Ignore 10ms delay for Braswell and Baytrail 2015-08-27 16:13 ` Kasagar, Srinidhi @ 2015-08-27 8:30 ` Mika Westerberg 2015-08-27 17:26 ` Rafael J. Wysocki 0 siblings, 1 reply; 15+ messages in thread From: Mika Westerberg @ 2015-08-27 8:30 UTC (permalink / raw) To: Kasagar, Srinidhi Cc: linux-acpi, rafael.j.wysocki, Kumar P Mahesh, Heikki Krogerus On Thu, Aug 27, 2015 at 09:43:21PM +0530, Kasagar, Srinidhi wrote: > >From 970c704dbb05aa8392b6e1090ceaf9ed76b6436b Mon Sep 17 00:00:00 2001 > From: Srinidhi Kasagar <srinidhi.kasagar@intel.com> > Date: Thu, 27 Aug 2015 21:30:55 +0530 > Subject: [PATCH] ACPI / LPSS: Ignore 10ms delay for Braswell > > LPSS devices in Braswell does not need the default 10ms > d3_delay imposed by PCI specification. Removing this > unnecessary delay significantly reduces the resume time > approximately upto 200ms on this platform. > > Signed-off-by: Srinidhi Kasagar <srinidhi.kasagar@intel.com> Looks good now, thanks! Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com> ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] ACPI / LPSS: Ignore 10ms delay for Braswell and Baytrail 2015-08-27 8:30 ` Mika Westerberg @ 2015-08-27 17:26 ` Rafael J. Wysocki 0 siblings, 0 replies; 15+ messages in thread From: Rafael J. Wysocki @ 2015-08-27 17:26 UTC (permalink / raw) To: Mika Westerberg Cc: Kasagar, Srinidhi, linux-acpi, rafael.j.wysocki, Kumar P Mahesh, Heikki Krogerus On Thursday, August 27, 2015 11:30:50 AM Mika Westerberg wrote: > On Thu, Aug 27, 2015 at 09:43:21PM +0530, Kasagar, Srinidhi wrote: > > >From 970c704dbb05aa8392b6e1090ceaf9ed76b6436b Mon Sep 17 00:00:00 2001 > > From: Srinidhi Kasagar <srinidhi.kasagar@intel.com> > > Date: Thu, 27 Aug 2015 21:30:55 +0530 > > Subject: [PATCH] ACPI / LPSS: Ignore 10ms delay for Braswell > > > > LPSS devices in Braswell does not need the default 10ms > > d3_delay imposed by PCI specification. Removing this > > unnecessary delay significantly reduces the resume time > > approximately upto 200ms on this platform. > > > > Signed-off-by: Srinidhi Kasagar <srinidhi.kasagar@intel.com> > > Looks good now, thanks! > > Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com> Applied, thanks! Rafael ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2015-08-27 16:58 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1440090968-17728-1-git-send-email-srinidhi.kasagar@intel.com>
2015-08-20 12:38 ` [PATCH] ACPI / LPSS: Ignore 10ms delay for Braswell and Baytrail Mika Westerberg
2015-08-20 13:04 ` Heikki Krogerus
2015-08-21 12:16 ` Kasagar, Srinidhi
2015-08-21 12:11 ` Kasagar, Srinidhi
2015-08-21 6:36 ` Mika Westerberg
2015-08-21 13:20 ` Mika Westerberg
2015-08-24 12:51 ` Kasagar, Srinidhi
2015-08-24 8:59 ` Mika Westerberg
2015-08-24 17:09 ` Kasagar, Srinidhi
2015-08-24 9:44 ` Mika Westerberg
2015-08-27 14:39 ` Kasagar, Srinidhi
2015-08-27 7:14 ` Mika Westerberg
2015-08-27 16:13 ` Kasagar, Srinidhi
2015-08-27 8:30 ` Mika Westerberg
2015-08-27 17:26 ` Rafael J. Wysocki
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).