diff mbox

[v4,2/3] PM / sleep: Go direct_complete if driver has no callbacks

Message ID 1436964008-15151-3-git-send-email-tomeu.vizoso@collabora.com (mailing list archive)
State Not Applicable, archived
Headers show

Commit Message

Tomeu Vizoso July 15, 2015, 12:40 p.m. UTC
If a suitable prepare callback cannot be found for a given device and
its driver has no PM callbacks at all, assume that it can go direct to
complete when the system goes to sleep.

The reason for this is that there's lots of devices in a system that do
no PM at all and there's no reason for them to prevent their ancestors
to do direct_complete if they can support it.

Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
---

 drivers/base/power/main.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

Comments

Alan Stern July 15, 2015, 6:47 p.m. UTC | #1
On Wed, 15 Jul 2015, Tomeu Vizoso wrote:

> If a suitable prepare callback cannot be found for a given device and
> its driver has no PM callbacks at all, assume that it can go direct to
> complete when the system goes to sleep.
> 
> The reason for this is that there's lots of devices in a system that do
> no PM at all and there's no reason for them to prevent their ancestors
> to do direct_complete if they can support it.
> 
> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
> ---
> 
>  drivers/base/power/main.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
> index 1710c26ba097..edda3f233c7c 100644
> --- a/drivers/base/power/main.c
> +++ b/drivers/base/power/main.c
> @@ -1540,6 +1540,21 @@ int dpm_suspend(pm_message_t state)
>  	return error;
>  }
>  
> +static bool driver_has_no_pm_callbacks(struct device_driver *drv)
> +{
> +	if (!drv->pm)
> +		return true;
> +
> +	return !drv->pm->prepare &&
> +	       !drv->pm->suspend &&
> +	       !drv->pm->suspend_late &&
> +	       !drv->pm->suspend_noirq &&
> +	       !drv->pm->resume_noirq &&
> +	       !drv->pm->resume_early &&
> +	       !drv->pm->resume &&
> +	       !drv->pm->complete;
> +}

This isn't exactly what I meant.  We also need to check the dev_pm_ops 
fields in dev->pm_domain, dev->type, dev->class, and dev->bus.  Only if 
_all_ of these callbacks are missing should we use direct_complete.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Rafael J. Wysocki July 16, 2015, 12:41 a.m. UTC | #2
On Wednesday, July 15, 2015 02:47:50 PM Alan Stern wrote:
> On Wed, 15 Jul 2015, Tomeu Vizoso wrote:
> 
> > If a suitable prepare callback cannot be found for a given device and
> > its driver has no PM callbacks at all, assume that it can go direct to
> > complete when the system goes to sleep.
> > 
> > The reason for this is that there's lots of devices in a system that do
> > no PM at all and there's no reason for them to prevent their ancestors
> > to do direct_complete if they can support it.
> > 
> > Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
> > ---
> > 
> >  drivers/base/power/main.c | 17 +++++++++++++++++
> >  1 file changed, 17 insertions(+)
> > 
> > diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
> > index 1710c26ba097..edda3f233c7c 100644
> > --- a/drivers/base/power/main.c
> > +++ b/drivers/base/power/main.c
> > @@ -1540,6 +1540,21 @@ int dpm_suspend(pm_message_t state)
> >  	return error;
> >  }
> >  
> > +static bool driver_has_no_pm_callbacks(struct device_driver *drv)
> > +{
> > +	if (!drv->pm)
> > +		return true;
> > +
> > +	return !drv->pm->prepare &&
> > +	       !drv->pm->suspend &&
> > +	       !drv->pm->suspend_late &&
> > +	       !drv->pm->suspend_noirq &&
> > +	       !drv->pm->resume_noirq &&
> > +	       !drv->pm->resume_early &&
> > +	       !drv->pm->resume &&
> > +	       !drv->pm->complete;
> > +}
> 
> This isn't exactly what I meant.  We also need to check the dev_pm_ops 
> fields in dev->pm_domain, dev->type, dev->class, and dev->bus.  Only if 
> _all_ of these callbacks are missing should we use direct_complete.

Also checking that on every suspend is kind of wasteful, because those things
do not change very often.

Thanks,
Rafael

--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tomeu Vizoso July 16, 2015, 8:47 a.m. UTC | #3
On 16 July 2015 at 02:41, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> On Wednesday, July 15, 2015 02:47:50 PM Alan Stern wrote:
>> On Wed, 15 Jul 2015, Tomeu Vizoso wrote:
>>
>> > If a suitable prepare callback cannot be found for a given device and
>> > its driver has no PM callbacks at all, assume that it can go direct to
>> > complete when the system goes to sleep.
>> >
>> > The reason for this is that there's lots of devices in a system that do
>> > no PM at all and there's no reason for them to prevent their ancestors
>> > to do direct_complete if they can support it.
>> >
>> > Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
>> > ---
>> >
>> >  drivers/base/power/main.c | 17 +++++++++++++++++
>> >  1 file changed, 17 insertions(+)
>> >
>> > diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
>> > index 1710c26ba097..edda3f233c7c 100644
>> > --- a/drivers/base/power/main.c
>> > +++ b/drivers/base/power/main.c
>> > @@ -1540,6 +1540,21 @@ int dpm_suspend(pm_message_t state)
>> >     return error;
>> >  }
>> >
>> > +static bool driver_has_no_pm_callbacks(struct device_driver *drv)
>> > +{
>> > +   if (!drv->pm)
>> > +           return true;
>> > +
>> > +   return !drv->pm->prepare &&
>> > +          !drv->pm->suspend &&
>> > +          !drv->pm->suspend_late &&
>> > +          !drv->pm->suspend_noirq &&
>> > +          !drv->pm->resume_noirq &&
>> > +          !drv->pm->resume_early &&
>> > +          !drv->pm->resume &&
>> > +          !drv->pm->complete;
>> > +}
>>
>> This isn't exactly what I meant.  We also need to check the dev_pm_ops
>> fields in dev->pm_domain, dev->type, dev->class, and dev->bus.  Only if
>> _all_ of these callbacks are missing should we use direct_complete.
>
> Also checking that on every suspend is kind of wasteful, because those things
> do not change very often.

Do you have any suggestion on when would be a good time to do that
check? device_pm_sleep_init() and device_pm_add() are unfortunately
too early.

Alternatively we could check once on the first suspend and cache it,
but I'm not sure that complexity would be worth it.

Thanks,

Tomeu

> Thanks,
> Rafael
>
> --
> 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/
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Rafael J. Wysocki July 17, 2015, 12:37 a.m. UTC | #4
On Thursday, July 16, 2015 10:47:51 AM Tomeu Vizoso wrote:
> On 16 July 2015 at 02:41, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> > On Wednesday, July 15, 2015 02:47:50 PM Alan Stern wrote:
> >> On Wed, 15 Jul 2015, Tomeu Vizoso wrote:
> >>
> >> > If a suitable prepare callback cannot be found for a given device and
> >> > its driver has no PM callbacks at all, assume that it can go direct to
> >> > complete when the system goes to sleep.
> >> >
> >> > The reason for this is that there's lots of devices in a system that do
> >> > no PM at all and there's no reason for them to prevent their ancestors
> >> > to do direct_complete if they can support it.
> >> >
> >> > Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
> >> > ---
> >> >
> >> >  drivers/base/power/main.c | 17 +++++++++++++++++
> >> >  1 file changed, 17 insertions(+)
> >> >
> >> > diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
> >> > index 1710c26ba097..edda3f233c7c 100644
> >> > --- a/drivers/base/power/main.c
> >> > +++ b/drivers/base/power/main.c
> >> > @@ -1540,6 +1540,21 @@ int dpm_suspend(pm_message_t state)
> >> >     return error;
> >> >  }
> >> >
> >> > +static bool driver_has_no_pm_callbacks(struct device_driver *drv)
> >> > +{
> >> > +   if (!drv->pm)
> >> > +           return true;
> >> > +
> >> > +   return !drv->pm->prepare &&
> >> > +          !drv->pm->suspend &&
> >> > +          !drv->pm->suspend_late &&
> >> > +          !drv->pm->suspend_noirq &&
> >> > +          !drv->pm->resume_noirq &&
> >> > +          !drv->pm->resume_early &&
> >> > +          !drv->pm->resume &&
> >> > +          !drv->pm->complete;
> >> > +}
> >>
> >> This isn't exactly what I meant.  We also need to check the dev_pm_ops
> >> fields in dev->pm_domain, dev->type, dev->class, and dev->bus.  Only if
> >> _all_ of these callbacks are missing should we use direct_complete.
> >
> > Also checking that on every suspend is kind of wasteful, because those things
> > do not change very often.
> 
> Do you have any suggestion on when would be a good time to do that
> check? device_pm_sleep_init() and device_pm_add() are unfortunately
> too early.

The time to check that is (a) when the device is registered (for the bus
type/class/device type), (b) when it is added to (or removed from) a PM
domain and (c) when a driver is bound to (unbound from) it.

> Alternatively we could check once on the first suspend and cache it,

That information may be stale by the time we suspend next time.

> but I'm not sure that complexity would be worth it.

I don't think that adding extra overhead to *every* suspend can be
justified easily, however.
diff mbox

Patch

diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
index 1710c26ba097..edda3f233c7c 100644
--- a/drivers/base/power/main.c
+++ b/drivers/base/power/main.c
@@ -1540,6 +1540,21 @@  int dpm_suspend(pm_message_t state)
 	return error;
 }
 
+static bool driver_has_no_pm_callbacks(struct device_driver *drv)
+{
+	if (!drv->pm)
+		return true;
+
+	return !drv->pm->prepare &&
+	       !drv->pm->suspend &&
+	       !drv->pm->suspend_late &&
+	       !drv->pm->suspend_noirq &&
+	       !drv->pm->resume_noirq &&
+	       !drv->pm->resume_early &&
+	       !drv->pm->resume &&
+	       !drv->pm->complete;
+}
+
 /**
  * device_prepare - Prepare a device for system power transition.
  * @dev: Device to handle.
@@ -1590,6 +1605,8 @@  static int device_prepare(struct device *dev, pm_message_t state)
 
 	if (callback)
 		ret = callback(dev);
+	else if (!dev->driver || driver_has_no_pm_callbacks(dev->driver))
+		ret = 1;	/* Let device go direct_complete */
 
 	device_unlock(dev);