Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1301337 > unrolled thread

Re: [PATCH] i2c/designware: enable i2c controller to suspend/resume asynchronously

Started byJarkko Nikula <jarkko.nikula@linux.intel.com>
First post2016-01-05 10:00 +0100
Last post2016-01-15 07:50 +0100
Articles 4 — 3 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  Re: [PATCH] i2c/designware: enable i2c controller to suspend/resume  asynchronously Jarkko Nikula <jarkko.nikula@linux.intel.com> - 2016-01-05 10:00 +0100
    Re: [PATCH] i2c/designware: enable i2c controller to suspend/resume  asynchronously Andy Shevchenko <andriy.shevchenko@linux.intel.com> - 2016-01-05 13:20 +0100
      Re: [PATCH] i2c/designware: enable i2c controller to suspend/resume  asynchronously "Fu, Zhonghui" <zhonghui.fu@linux.intel.com> - 2016-01-15 08:10 +0100
    Re: [PATCH] i2c/designware: enable i2c controller to suspend/resume  asynchronously "Fu, Zhonghui" <zhonghui.fu@linux.intel.com> - 2016-01-15 07:50 +0100

#1301337 — Re: [PATCH] i2c/designware: enable i2c controller to suspend/resume asynchronously

FromJarkko Nikula <jarkko.nikula@linux.intel.com>
Date2016-01-05 10:00 +0100
SubjectRe: [PATCH] i2c/designware: enable i2c controller to suspend/resume asynchronously
Message-ID<qNvDc-84L-17@gated-at.bofh.it>
Hi

On 12/24/2015 04:30 PM, Fu, Zhonghui wrote:
> Now, PM core supports asynchronous suspend/resume mode for devices
> during system suspend/resume, and the power state transition of one
> device may be completed in separate kernel thread. PM core ensures
> all power state transition dependency between devices. This patch
> enables designware i2c controllers to suspend/resume asynchronously.
> This will take advantage of multicore and improve system suspend/resume
> speed. After enabling all i2c devices, i2c adapters and i2c controllers
> on ASUS T100TA tablet, the system suspend-to-idle time is reduced to
> about 510ms from 750ms, and the system resume time is reduced to about
> 790ms from 900ms.
>
Nice reduction :-)

> diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
> index 6b00061..395130b 100644
> --- a/drivers/i2c/busses/i2c-designware-platdrv.c
> +++ b/drivers/i2c/busses/i2c-designware-platdrv.c
> @@ -230,6 +230,7 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
>   	}
>
>   	adap = &dev->adapter;
> +	device_enable_async_suspend(&pdev->dev);
>   	adap->owner = THIS_MODULE;
>   	adap->class = I2C_CLASS_DEPRECATED;
>   	ACPI_COMPANION_SET(&adap->dev, ACPI_COMPANION(&pdev->dev));

Does device_enable_async_suspend() need to be called before enabling 
runtime PM? I suppose not since there appears to have also related sysfs 
node for toggling it runtime.

I'm thinking if you could move the device_enable_async_suspend() call 
into drivers/i2c/busses/i2c-designware-core.c: i2c_dw_probe() and then 
also PCI enumerated adapter could take advantage of it.

-- 
Jarkko
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]


#1301480

FromAndy Shevchenko <andriy.shevchenko@linux.intel.com>
Date2016-01-05 13:20 +0100
Message-ID<qNyKJ-2Cw-1@gated-at.bofh.it>
In reply to#1301337
On Tue, 2016-01-05 at 10:53 +0200, Jarkko Nikula wrote:
> Hi
> 
> On 12/24/2015 04:30 PM, Fu, Zhonghui wrote:
> > Now, PM core supports asynchronous suspend/resume mode for devices
> > during system suspend/resume, and the power state transition of one
> > device may be completed in separate kernel thread. PM core ensures
> > all power state transition dependency between devices. This patch
> > enables designware i2c controllers to suspend/resume
> > asynchronously.
> > This will take advantage of multicore and improve system
> > suspend/resume
> > speed. After enabling all i2c devices, i2c adapters and i2c
> > controllers
> > on ASUS T100TA tablet, the system suspend-to-idle time is reduced
> > to
> > about 510ms from 750ms, and the system resume time is reduced to
> > about
> > 790ms from 900ms.
> > 
> Nice reduction :-)
> 
> > diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c
> > b/drivers/i2c/busses/i2c-designware-platdrv.c
> > index 6b00061..395130b 100644
> > --- a/drivers/i2c/busses/i2c-designware-platdrv.c
> > +++ b/drivers/i2c/busses/i2c-designware-platdrv.c
> > @@ -230,6 +230,7 @@ static int dw_i2c_plat_probe(struct
> > platform_device *pdev)
> >   	}
> > 
> >   	adap = &dev->adapter;
> > +	device_enable_async_suspend(&pdev->dev);
> >   	adap->owner = THIS_MODULE;
> >   	adap->class = I2C_CLASS_DEPRECATED;
> >   	ACPI_COMPANION_SET(&adap->dev, ACPI_COMPANION(&pdev-
> > >dev));
> 
> Does device_enable_async_suspend() need to be called before enabling 
> runtime PM? I suppose not since there appears to have also related
> sysfs 
> node for toggling it runtime.
> 
> I'm thinking if you could move the device_enable_async_suspend() call
> into drivers/i2c/busses/i2c-designware-core.c: i2c_dw_probe() and
> then 
> also PCI enumerated adapter could take advantage of it.

I concern about Intel BayTrail-T / Braswell / CherryTrail cases, since
we have non-trivial PM for LPSS there. Zhonghui, have you a chance to
stress test this on platforms based on mentioned SoCs?


-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1309920

From"Fu, Zhonghui" <zhonghui.fu@linux.intel.com>
Date2016-01-15 08:10 +0100
Message-ID<qR6Gd-2DL-17@gated-at.bofh.it>
In reply to#1301480

On 1/5/2016 8:14 PM, Andy Shevchenko wrote:
> On Tue, 2016-01-05 at 10:53 +0200, Jarkko Nikula wrote:
>> Hi
>>
>> On 12/24/2015 04:30 PM, Fu, Zhonghui wrote:
>>> Now, PM core supports asynchronous suspend/resume mode for devices
>>> during system suspend/resume, and the power state transition of one
>>> device may be completed in separate kernel thread. PM core ensures
>>> all power state transition dependency between devices. This patch
>>> enables designware i2c controllers to suspend/resume
>>> asynchronously.
>>> This will take advantage of multicore and improve system
>>> suspend/resume
>>> speed. After enabling all i2c devices, i2c adapters and i2c
>>> controllers
>>> on ASUS T100TA tablet, the system suspend-to-idle time is reduced
>>> to
>>> about 510ms from 750ms, and the system resume time is reduced to
>>> about
>>> 790ms from 900ms.
>>>
>> Nice reduction :-)
>>
>>> diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c
>>> b/drivers/i2c/busses/i2c-designware-platdrv.c
>>> index 6b00061..395130b 100644
>>> --- a/drivers/i2c/busses/i2c-designware-platdrv.c
>>> +++ b/drivers/i2c/busses/i2c-designware-platdrv.c
>>> @@ -230,6 +230,7 @@ static int dw_i2c_plat_probe(struct
>>> platform_device *pdev)
>>>   	}
>>>
>>>   	adap = &dev->adapter;
>>> +	device_enable_async_suspend(&pdev->dev);
>>>   	adap->owner = THIS_MODULE;
>>>   	adap->class = I2C_CLASS_DEPRECATED;
>>>   	ACPI_COMPANION_SET(&adap->dev, ACPI_COMPANION(&pdev-
>>>> dev));
>> Does device_enable_async_suspend() need to be called before enabling 
>> runtime PM? I suppose not since there appears to have also related
>> sysfs 
>> node for toggling it runtime.
>>
>> I'm thinking if you could move the device_enable_async_suspend() call
>> into drivers/i2c/busses/i2c-designware-core.c: i2c_dw_probe() and
>> then 
>> also PCI enumerated adapter could take advantage of it.
> I concern about Intel BayTrail-T / Braswell / CherryTrail cases, since
> we have non-trivial PM for LPSS there. Zhonghui, have you a chance to
> stress test this on platforms based on mentioned SoCs?
Because of long leave, so sorry for late reply.

I understand what you said, if enable all LPSS devices suspend/resume asynchronously, the system can't resume sometimes on ASUS T100TA(BayTrail-T SoC). But, I have verified that the system can resume normally every time if enable only i2c controller async mode and let other LPSS devices in sync mode on ASUS T100TA.  I have no Braswell and cherryTrail platforms, so no verification test on them.

I have submitted a new version of this patch to move the device_enable_async_suspend() call into i2c_dw_proble() function - "[PATCH v2] i2c/designware: enable i2c controller to suspend/resume asynchronously".


Thanks,
Zhonghui
>
>

[toc] | [prev] | [next] | [standalone]


#1309895

From"Fu, Zhonghui" <zhonghui.fu@linux.intel.com>
Date2016-01-15 07:50 +0100
Message-ID<qR6mR-2gY-3@gated-at.bofh.it>
In reply to#1301337

On 1/5/2016 4:53 PM, Jarkko Nikula wrote:
> Hi
>
> On 12/24/2015 04:30 PM, Fu, Zhonghui wrote:
>> Now, PM core supports asynchronous suspend/resume mode for devices
>> during system suspend/resume, and the power state transition of one
>> device may be completed in separate kernel thread. PM core ensures
>> all power state transition dependency between devices. This patch
>> enables designware i2c controllers to suspend/resume asynchronously.
>> This will take advantage of multicore and improve system suspend/resume
>> speed. After enabling all i2c devices, i2c adapters and i2c controllers
>> on ASUS T100TA tablet, the system suspend-to-idle time is reduced to
>> about 510ms from 750ms, and the system resume time is reduced to about
>> 790ms from 900ms.
>>
> Nice reduction :-)
>
>> diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
>> index 6b00061..395130b 100644
>> --- a/drivers/i2c/busses/i2c-designware-platdrv.c
>> +++ b/drivers/i2c/busses/i2c-designware-platdrv.c
>> @@ -230,6 +230,7 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
>>       }
>>
>>       adap = &dev->adapter;
>> +    device_enable_async_suspend(&pdev->dev);
>>       adap->owner = THIS_MODULE;
>>       adap->class = I2C_CLASS_DEPRECATED;
>>       ACPI_COMPANION_SET(&adap->dev, ACPI_COMPANION(&pdev->dev));
>
> Does device_enable_async_suspend() need to be called before enabling runtime PM? I suppose not since there appears to have also related sysfs node for toggling it runtime.
>
> I'm thinking if you could move the device_enable_async_suspend() call into drivers/i2c/busses/i2c-designware-core.c: i2c_dw_probe() and then also PCI enumerated adapter could take advantage of it.

Because of long leave, so sorry for late reply.

Your proposal is right. I have submitted a new patch according to your comments - "[PATCH v2] i2c/designware: enable i2c controller to suspend/resume asynchronously".


Thanks,
Zhonghui

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web