[PATCH/RESEND] soc-camera: add runtime pm support for subdevices

[PATCH/RESEND] soc-camera: add runtime pm support for subdevices

[Guennadi Liakhovetski]

To save power soc-camera powers subdevices down, when they are not in use, 
if this is supported by the platform. However, the V4L standard dictates, 
that video nodes shall preserve configuration between uses. This requires 
runtime power management, which is implemented by this patch. It allows 
subdevice drivers to specify their runtime power-management methods, by 
assigning a type to the video device.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---

I've posted this patch to linux-media earlier, but I'd also like to get 
comments on linux-pm, sorry to linux-media falks for a duplicate. To 
explain a bit - soc_camera.c is a management module, that binds video 
interfaces on SoCs and sensor drivers. The calls, that I am adding to 
soc_camera.c shall save and restore sensor registers before they are 
powered down and after they are powered up.

diff --git a/drivers/media/video/soc_camera.c b/drivers/media/video/soc_camera.c
index 6b3fbcc..53201f3 100644
--- a/drivers/media/video/soc_camera.c
+++ b/drivers/media/video/soc_camera.c
@@ -24,6 +24,7 @@
 #include <linux/mutex.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
 #include <linux/vmalloc.h>
 
 #include <media/soc_camera.h>
@@ -387,6 +388,11 @@ static int soc_camera_open(struct file *file)
 			goto eiciadd;
 		}
 
+		pm_runtime_enable(&icd->vdev->dev);
+		ret = pm_runtime_resume(&icd->vdev->dev);
+		if (ret < 0 && ret != -ENOSYS)
+			goto eresume;
+
 		/*
 		 * Try to configure with default parameters. Notice: this is the
 		 * very first open, so, we cannot race against other calls,
@@ -408,10 +414,12 @@ static int soc_camera_open(struct file *file)
 	return 0;
 
 	/*
-	 * First five errors are entered with the .video_lock held
+	 * First four errors are entered with the .video_lock held
 	 * and use_count == 1
 	 */
 esfmt:
+	pm_runtime_disable(&icd->vdev->dev);
+eresume:
 	ici->ops->remove(icd);
 eiciadd:
 	if (icl->power)
@@ -436,7 +444,11 @@ static int soc_camera_close(struct file *file)
 	if (!icd->use_count) {
 		struct soc_camera_link *icl = to_soc_camera_link(icd);
 
+		pm_runtime_suspend(&icd->vdev->dev);
+		pm_runtime_disable(&icd->vdev->dev);
+
 		ici->ops->remove(icd);
+
 		if (icl->power)
 			icl->power(icd->pdev, 0);
 	}
@@ -1294,6 +1306,7 @@ static int video_dev_create(struct soc_camera_device *icd)
  */
 static int soc_camera_video_start(struct soc_camera_device *icd)
 {
+	struct device_type *type = icd->vdev->dev.type;
 	int ret;
 
 	if (!icd->dev.parent)
@@ -1310,6 +1323,9 @@ static int soc_camera_video_start(struct soc_camera_device *icd)
 		return ret;
 	}
 
+	/* Restore device type, possibly set by the subdevice driver */
+	icd->vdev->dev.type = type;
+
 	return 0;
 }
 
diff --git a/include/media/soc_camera.h b/include/media/soc_camera.h
index dcc5b86..58b39a9 100644
--- a/include/media/soc_camera.h
+++ b/include/media/soc_camera.h
@@ -282,4 +282,12 @@ static inline void soc_camera_limit_side(unsigned int *start,
 extern unsigned long soc_camera_apply_sensor_flags(struct soc_camera_link *icl,
 						   unsigned long flags);
 
+/* This is only temporary here - until v4l2-subdev begins to link to video_device */
+#include <linux/i2c.h>
+static inline struct video_device *soc_camera_i2c_to_vdev(struct i2c_client *client)
+{
+	struct soc_camera_device *icd = client->dev.platform_data;
+	return icd->vdev;
+}
+
 #endif

Re: [PATCH/RESEND] soc-camera: add runtime pm support for subdevices

[Mauro Carvalho Chehab]

Guennadi Liakhovetski wrote:
> To save power soc-camera powers subdevices down, when they are not in use, 
> if this is supported by the platform. However, the V4L standard dictates, 
> that video nodes shall preserve configuration between uses. This requires 
> runtime power management, which is implemented by this patch. It allows 
> subdevice drivers to specify their runtime power-management methods, by 
> assigning a type to the video device.

It seems a great idea to me. For sure we need some sort of power management
control.

> 
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> ---
> 
> I've posted this patch to linux-media earlier, but I'd also like to get 
> comments on linux-pm, sorry to linux-media falks for a duplicate. To 
> explain a bit - soc_camera.c is a management module, that binds video 
> interfaces on SoCs and sensor drivers. The calls, that I am adding to 
> soc_camera.c shall save and restore sensor registers before they are 
> powered down and after they are powered up.
> 
> diff --git a/drivers/media/video/soc_camera.c b/drivers/media/video/soc_camera.c
> index 6b3fbcc..53201f3 100644
> --- a/drivers/media/video/soc_camera.c
> +++ b/drivers/media/video/soc_camera.c
> @@ -24,6 +24,7 @@
>  #include <linux/mutex.h>
>  #include <linux/module.h>
>  #include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
>  #include <linux/vmalloc.h>


Hmm... wouldn't it be better to enable it at the subsystem level? We may for 
example call ?
The subsystem can call vidioc_streamoff() at suspend and vidioc_streamon() at
resume, if the device were streaming during suspend. We may add another ops to
the struct for the drivers/subdrivers that needs additional care.

That's said, it shouldn't be hard to implement some routine that will save/restore
all registers if the device goes to power down mode. Unfortunately, very few
devices successfully recovers from hibernation if streaming. One good example
is saa7134, that even disables/re-enables IR IRQ's during suspend/resume.

-- 

Cheers,
Mauro

Re: [PATCH/RESEND] soc-camera: add runtime pm support for subdevices

[Guennadi Liakhovetski]

Hi Mauro

Thanks for your comments.

On Mon, 8 Feb 2010, Mauro Carvalho Chehab wrote:

> Guennadi Liakhovetski wrote:
> > To save power soc-camera powers subdevices down, when they are not in use, 
> > if this is supported by the platform. However, the V4L standard dictates, 
> > that video nodes shall preserve configuration between uses. This requires 
> > runtime power management, which is implemented by this patch. It allows 
> > subdevice drivers to specify their runtime power-management methods, by 
> > assigning a type to the video device.
> 
> It seems a great idea to me. For sure we need some sort of power management
> control.

Agree;)

> > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > ---
> > 
> > I've posted this patch to linux-media earlier, but I'd also like to get 
> > comments on linux-pm, sorry to linux-media falks for a duplicate. To 
> > explain a bit - soc_camera.c is a management module, that binds video 
> > interfaces on SoCs and sensor drivers. The calls, that I am adding to 
> > soc_camera.c shall save and restore sensor registers before they are 
> > powered down and after they are powered up.
> > 
> > diff --git a/drivers/media/video/soc_camera.c b/drivers/media/video/soc_camera.c
> > index 6b3fbcc..53201f3 100644
> > --- a/drivers/media/video/soc_camera.c
> > +++ b/drivers/media/video/soc_camera.c
> > @@ -24,6 +24,7 @@
> >  #include <linux/mutex.h>
> >  #include <linux/module.h>
> >  #include <linux/platform_device.h>
> > +#include <linux/pm_runtime.h>
> >  #include <linux/vmalloc.h>
> 
> 
> Hmm... wouldn't it be better to enable it at the subsystem level? We may for 
> example call ?
> The subsystem can call vidioc_streamoff() at suspend and vidioc_streamon() at
> resume, if the device were streaming during suspend. We may add another ops to
> the struct for the drivers/subdrivers that needs additional care.
> 
> That's said, it shouldn't be hard to implement some routine that will save/restore
> all registers if the device goes to power down mode. Unfortunately, very few
> devices successfully recovers from hibernation if streaming. One good example
> is saa7134, that even disables/re-enables IR IRQ's during suspend/resume.

To clarify a bit - this patch implements not static PM, but dynamic 
(runtime) power-management. In this case it means, we are trying to save 
power while the system is running, but we know, that the sensor is not 
needed. Specifically, as long as no application is holding the video 
device open. And this information is only available at the bridge driver 
(soc-camera core) level - there is no subdev operation for open and close 
calls, so, subdevices do not "know" whether they are in use or not. So, 
only saving / restoring registers when streaming is not enough. Static PM 
will also be interesting - as it has been mentioned before, we will have 
to be careful, because sensors "sit" on two busses - i2c and video. So, 
you have to resume after both are up and suspend before the first of them 
goes down... So, that will be a different exciting topic;)

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [PATCH/RESEND] soc-camera: add runtime pm support for subdevices

[Mauro Carvalho Chehab]

Guennadi Liakhovetski wrote:
> Hi Mauro
> 
> Thanks for your comments.
> 
> On Mon, 8 Feb 2010, Mauro Carvalho Chehab wrote:
> 
>> Guennadi Liakhovetski wrote:
>>> To save power soc-camera powers subdevices down, when they are not in use, 
>>> if this is supported by the platform. However, the V4L standard dictates, 
>>> that video nodes shall preserve configuration between uses. This requires 
>>> runtime power management, which is implemented by this patch. It allows 
>>> subdevice drivers to specify their runtime power-management methods, by 
>>> assigning a type to the video device.
>> It seems a great idea to me. For sure we need some sort of power management
>> control.
> 
> Agree;)
> 
>>> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
>>> ---
>>>
>>> I've posted this patch to linux-media earlier, but I'd also like to get 
>>> comments on linux-pm, sorry to linux-media falks for a duplicate. To 
>>> explain a bit - soc_camera.c is a management module, that binds video 
>>> interfaces on SoCs and sensor drivers. The calls, that I am adding to 
>>> soc_camera.c shall save and restore sensor registers before they are 
>>> powered down and after they are powered up.
>>>
>>> diff --git a/drivers/media/video/soc_camera.c b/drivers/media/video/soc_camera.c
>>> index 6b3fbcc..53201f3 100644
>>> --- a/drivers/media/video/soc_camera.c
>>> +++ b/drivers/media/video/soc_camera.c
>>> @@ -24,6 +24,7 @@
>>>  #include <linux/mutex.h>
>>>  #include <linux/module.h>
>>>  #include <linux/platform_device.h>
>>> +#include <linux/pm_runtime.h>
>>>  #include <linux/vmalloc.h>
>>
>> Hmm... wouldn't it be better to enable it at the subsystem level? We may for 
>> example call ?
>> The subsystem can call vidioc_streamoff() at suspend and vidioc_streamon() at
>> resume, if the device were streaming during suspend. We may add another ops to
>> the struct for the drivers/subdrivers that needs additional care.
>>
>> That's said, it shouldn't be hard to implement some routine that will save/restore
>> all registers if the device goes to power down mode. Unfortunately, very few
>> devices successfully recovers from hibernation if streaming. One good example
>> is saa7134, that even disables/re-enables IR IRQ's during suspend/resume.
> 
> To clarify a bit - this patch implements not static PM, but dynamic 
> (runtime) power-management. 

Ok.

> In this case it means, we are trying to save 
> power while the system is running, but we know, that the sensor is not 
> needed. Specifically, as long as no application is holding the video 
> device open. And this information is only available at the bridge driver 
> (soc-camera core) level - there is no subdev operation for open and close 
> calls, so, subdevices do not "know" whether they are in use or not. So, 
> only saving / restoring registers when streaming is not enough. Static PM 
> will also be interesting - as it has been mentioned before, we will have 
> to be careful, because sensors "sit" on two busses - i2c and video. So, 
> you have to resume after both are up and suspend before the first of them 
> goes down... So, that will be a different exciting topic;)

In fact, on all drivers, there are devices that needs to be turn on only when
streaming is happening: sensors, analog TV/audio demods, digital demods. Also,
a few devices (for example: TV tuners) could eventually be on power off when
no device is opened.

As the V4L core knows when this is happening (due to
open/close/poll/streamon/reqbuf/qbuf/dqbuf hooks, I think the runtime management 
can happen at V4L core level.

> 
> Thanks
> Guennadi
> ---
> Guennadi Liakhovetski, Ph.D.
> Freelance Open-Source Software Developer
> http://www.open-technology.de/


-- 

Cheers,
Mauro

Re: [linux-pm] [PATCH/RESEND] soc-camera: add runtime pm support for subdevices

[Rafael J. Wysocki]

On Monday 08 February 2010, Guennadi Liakhovetski wrote:
> To save power soc-camera powers subdevices down, when they are not in use, 
> if this is supported by the platform. However, the V4L standard dictates, 
> that video nodes shall preserve configuration between uses. This requires 
> runtime power management, which is implemented by this patch. It allows 
> subdevice drivers to specify their runtime power-management methods, by 
> assigning a type to the video device.

You need a support for that at the bus type/device type/device class level,
because the core doesn't execute the driver callbacks directly.

Rafael

> 
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> ---
> 
> I've posted this patch to linux-media earlier, but I'd also like to get 
> comments on linux-pm, sorry to linux-media falks for a duplicate. To 
> explain a bit - soc_camera.c is a management module, that binds video 
> interfaces on SoCs and sensor drivers. The calls, that I am adding to 
> soc_camera.c shall save and restore sensor registers before they are 
> powered down and after they are powered up.
> 
> diff --git a/drivers/media/video/soc_camera.c b/drivers/media/video/soc_camera.c
> index 6b3fbcc..53201f3 100644
> --- a/drivers/media/video/soc_camera.c
> +++ b/drivers/media/video/soc_camera.c
> @@ -24,6 +24,7 @@
>  #include <linux/mutex.h>
>  #include <linux/module.h>
>  #include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
>  #include <linux/vmalloc.h>
>  
>  #include <media/soc_camera.h>
> @@ -387,6 +388,11 @@ static int soc_camera_open(struct file *file)
>  			goto eiciadd;
>  		}
>  
> +		pm_runtime_enable(&icd->vdev->dev);
> +		ret = pm_runtime_resume(&icd->vdev->dev);
> +		if (ret < 0 && ret != -ENOSYS)
> +			goto eresume;
> +
>  		/*
>  		 * Try to configure with default parameters. Notice: this is the
>  		 * very first open, so, we cannot race against other calls,
> @@ -408,10 +414,12 @@ static int soc_camera_open(struct file *file)
>  	return 0;
>  
>  	/*
> -	 * First five errors are entered with the .video_lock held
> +	 * First four errors are entered with the .video_lock held
>  	 * and use_count == 1
>  	 */
>  esfmt:
> +	pm_runtime_disable(&icd->vdev->dev);
> +eresume:
>  	ici->ops->remove(icd);
>  eiciadd:
>  	if (icl->power)
> @@ -436,7 +444,11 @@ static int soc_camera_close(struct file *file)
>  	if (!icd->use_count) {
>  		struct soc_camera_link *icl = to_soc_camera_link(icd);
>  
> +		pm_runtime_suspend(&icd->vdev->dev);
> +		pm_runtime_disable(&icd->vdev->dev);
> +
>  		ici->ops->remove(icd);
> +
>  		if (icl->power)
>  			icl->power(icd->pdev, 0);
>  	}
> @@ -1294,6 +1306,7 @@ static int video_dev_create(struct soc_camera_device *icd)
>   */
>  static int soc_camera_video_start(struct soc_camera_device *icd)
>  {
> +	struct device_type *type = icd->vdev->dev.type;
>  	int ret;
>  
>  	if (!icd->dev.parent)
> @@ -1310,6 +1323,9 @@ static int soc_camera_video_start(struct soc_camera_device *icd)
>  		return ret;
>  	}
>  
> +	/* Restore device type, possibly set by the subdevice driver */
> +	icd->vdev->dev.type = type;
> +
>  	return 0;
>  }
>  
> diff --git a/include/media/soc_camera.h b/include/media/soc_camera.h
> index dcc5b86..58b39a9 100644
> --- a/include/media/soc_camera.h
> +++ b/include/media/soc_camera.h
> @@ -282,4 +282,12 @@ static inline void soc_camera_limit_side(unsigned int *start,
>  extern unsigned long soc_camera_apply_sensor_flags(struct soc_camera_link *icl,
>  						   unsigned long flags);
>  
> +/* This is only temporary here - until v4l2-subdev begins to link to video_device */
> +#include <linux/i2c.h>
> +static inline struct video_device *soc_camera_i2c_to_vdev(struct i2c_client *client)
> +{
> +	struct soc_camera_device *icd = client->dev.platform_data;
> +	return icd->vdev;
> +}
> +
>  #endif
> _______________________________________________

Re: [linux-pm] [PATCH/RESEND] soc-camera: add runtime pm support for subdevices

[Guennadi Liakhovetski]

On Mon, 8 Feb 2010, Rafael J. Wysocki wrote:

> On Monday 08 February 2010, Guennadi Liakhovetski wrote:
> > To save power soc-camera powers subdevices down, when they are not in use, 
> > if this is supported by the platform. However, the V4L standard dictates, 
> > that video nodes shall preserve configuration between uses. This requires 
> > runtime power management, which is implemented by this patch. It allows 
> > subdevice drivers to specify their runtime power-management methods, by 
> > assigning a type to the video device.
> 
> You need a support for that at the bus type/device type/device class level,
> because the core doesn't execute the driver callbacks directly.

That's exactly what this patch is doing - adding a device type with pm 
callbacks. What I wasn't sure about - and why I posted here - is whether 
that's a proper way to use pm_runtime_resume() and pm_runtime_suspend() 
calls - from a central module, targeting underlying devices, before 
powering them down and after powering them up?

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [PATCH/RESEND] soc-camera: add runtime pm support for subdevices

[Guennadi Liakhovetski]

On Mon, 8 Feb 2010, Mauro Carvalho Chehab wrote:

> In fact, on all drivers, there are devices that needs to be turn on only when
> streaming is happening: sensors, analog TV/audio demods, digital demods. Also,
> a few devices (for example: TV tuners) could eventually be on power off when
> no device is opened.
> 
> As the V4L core knows when this is happening (due to
> open/close/poll/streamon/reqbuf/qbuf/dqbuf hooks, I think the runtime management 
> can happen at V4L core level.

Well, we can move it up to v4l core. Should it get any more complicated 
than adding

	ret = pm_runtime_resume(&vdev->dev);
	if (ret < 0 && ret != -ENOSYS)
		return ret;

to v4l2_open() and

	pm_runtime_suspend(&vdev->dev);

to v4l2_release()? And to agree, that video drivers may set a device type 
to implement runtime PM, and that the v4l core shouldn't touch it? Then, 
for example, a bridge driver could implement such a device type instance 
and suspend or resume all related components?

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [PATCH/RESEND] soc-camera: add runtime pm support for subdevices

[Hans Verkuil]

> On Mon, 8 Feb 2010, Mauro Carvalho Chehab wrote:
>
>> In fact, on all drivers, there are devices that needs to be turn on only
>> when
>> streaming is happening: sensors, analog TV/audio demods, digital demods.
>> Also,
>> a few devices (for example: TV tuners) could eventually be on power off
>> when
>> no device is opened.
>>
>> As the V4L core knows when this is happening (due to
>> open/close/poll/streamon/reqbuf/qbuf/dqbuf hooks, I think the runtime
>> management
>> can happen at V4L core level.
>
> Well, we can move it up to v4l core. Should it get any more complicated
> than adding
>
> 	ret = pm_runtime_resume(&vdev->dev);
> 	if (ret < 0 && ret != -ENOSYS)
> 		return ret;
>
> to v4l2_open() and
>
> 	pm_runtime_suspend(&vdev->dev);
>
> to v4l2_release()?

My apologies if I say something stupid as I know little about pm: are you
assuming here that streaming only happens on one device node? That may be
true for soc-camera, but other devices can have multiple streaming nodes
(video, vbi, mpeg, etc). So the call to v4l2_release does not necessarily
mean that streaming has stopped.

Regards,

      Hans

> And to agree, that video drivers may set a device type
> to implement runtime PM, and that the v4l core shouldn't touch it? Then,
> for example, a bridge driver could implement such a device type instance
> and suspend or resume all related components?
>
> Thanks
> Guennadi
> ---
> Guennadi Liakhovetski, Ph.D.
> Freelance Open-Source Software Developer
> http://www.open-technology.de/
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>


-- 
Hans Verkuil - video4linux developer - sponsored by TANDBERG Telecom

Re: [PATCH/RESEND] soc-camera: add runtime pm support for subdevices

[Guennadi Liakhovetski]

On Tue, 9 Feb 2010, Hans Verkuil wrote:

> 
> > On Mon, 8 Feb 2010, Mauro Carvalho Chehab wrote:
> >
> >> In fact, on all drivers, there are devices that needs to be turn on only
> >> when
> >> streaming is happening: sensors, analog TV/audio demods, digital demods.
> >> Also,
> >> a few devices (for example: TV tuners) could eventually be on power off
> >> when
> >> no device is opened.
> >>
> >> As the V4L core knows when this is happening (due to
> >> open/close/poll/streamon/reqbuf/qbuf/dqbuf hooks, I think the runtime
> >> management
> >> can happen at V4L core level.
> >
> > Well, we can move it up to v4l core. Should it get any more complicated
> > than adding
> >
> > 	ret = pm_runtime_resume(&vdev->dev);
> > 	if (ret < 0 && ret != -ENOSYS)
> > 		return ret;
> >
> > to v4l2_open() and
> >
> > 	pm_runtime_suspend(&vdev->dev);
> >
> > to v4l2_release()?
> 
> My apologies if I say something stupid as I know little about pm: are you
> assuming here that streaming only happens on one device node? That may be
> true for soc-camera, but other devices can have multiple streaming nodes
> (video, vbi, mpeg, etc). So the call to v4l2_release does not necessarily
> mean that streaming has stopped.

Of course you're right, and it concerns not only multiple streaming modes, 
but simple cases of multiple openings of one node. I was too fast to 
transfer the implementation from soc-camera to v4l2 - in soc-camera I'm 
counting opens and closes and only calling pm hooks on first open and last 
close. So, if we want to put it in v4l-core, we'd have to do something 
similar, I presume.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [PATCH/RESEND] soc-camera: add runtime pm support for subdevices

[Hans Verkuil]

> On Tue, 9 Feb 2010, Hans Verkuil wrote:
>
>>
>> > On Mon, 8 Feb 2010, Mauro Carvalho Chehab wrote:
>> >
>> >> In fact, on all drivers, there are devices that needs to be turn on
>> only
>> >> when
>> >> streaming is happening: sensors, analog TV/audio demods, digital
>> demods.
>> >> Also,
>> >> a few devices (for example: TV tuners) could eventually be on power
>> off
>> >> when
>> >> no device is opened.
>> >>
>> >> As the V4L core knows when this is happening (due to
>> >> open/close/poll/streamon/reqbuf/qbuf/dqbuf hooks, I think the runtime
>> >> management
>> >> can happen at V4L core level.
>> >
>> > Well, we can move it up to v4l core. Should it get any more
>> complicated
>> > than adding
>> >
>> > 	ret = pm_runtime_resume(&vdev->dev);
>> > 	if (ret < 0 && ret != -ENOSYS)
>> > 		return ret;
>> >
>> > to v4l2_open() and
>> >
>> > 	pm_runtime_suspend(&vdev->dev);
>> >
>> > to v4l2_release()?
>>
>> My apologies if I say something stupid as I know little about pm: are
>> you
>> assuming here that streaming only happens on one device node? That may
>> be
>> true for soc-camera, but other devices can have multiple streaming nodes
>> (video, vbi, mpeg, etc). So the call to v4l2_release does not
>> necessarily
>> mean that streaming has stopped.
>
> Of course you're right, and it concerns not only multiple streaming modes,
> but simple cases of multiple openings of one node. I was too fast to
> transfer the implementation from soc-camera to v4l2 - in soc-camera I'm
> counting opens and closes and only calling pm hooks on first open and last
> close. So, if we want to put it in v4l-core, we'd have to do something
> similar, I presume.

I wouldn't mind having such counters. There are more situations where
knowing whether it is the first open or last close comes in handy.

However, in general I think that pm shouldn't be done in the core. It is
just too hardware dependent. E.g. there may both capture and display video
nodes in the driver. And when the last capture stops you can for example
power down the receiver chips. The same with display and transmitter
chips. But if both are controlled by the same driver, then a general open
counter will not work either.

But if you have ideas to improve the core to make it easier to add pm
support to the drivers that need it, then I am all for it.

Regards,

        Hans

>
> Thanks
> Guennadi
> ---
> Guennadi Liakhovetski, Ph.D.
> Freelance Open-Source Software Developer
> http://www.open-technology.de/
>


-- 
Hans Verkuil - video4linux developer - sponsored by TANDBERG Telecom

Re: [PATCH/RESEND] soc-camera: add runtime pm support for subdevices

[Guennadi Liakhovetski]

On Tue, 9 Feb 2010, Hans Verkuil wrote:

> > Of course you're right, and it concerns not only multiple streaming modes,
> > but simple cases of multiple openings of one node. I was too fast to
> > transfer the implementation from soc-camera to v4l2 - in soc-camera I'm
> > counting opens and closes and only calling pm hooks on first open and last
> > close. So, if we want to put it in v4l-core, we'd have to do something
> > similar, I presume.
> 
> I wouldn't mind having such counters. There are more situations where
> knowing whether it is the first open or last close comes in handy.
> 
> However, in general I think that pm shouldn't be done in the core. It is
> just too hardware dependent. E.g. there may both capture and display video
> nodes in the driver. And when the last capture stops you can for example
> power down the receiver chips.

In this example, don't you have two struct video_device instances, so, 
last close of one of them will call pm_runtime_suspend() for _that_ 
device, without affecting the other?

> The same with display and transmitter
> chips. But if both are controlled by the same driver, then a general open
> counter will not work either.

Well, first, you'd get a counter per video_device, not per driver, but 
you, probably mean cases with multiple "entities" attached to one device 
node. But even in this case, your suspend is called, and you can decide, 
whether you actually can suspend and suspend all children, or fail and 
return -EBUSY... I was expecting to implement these pm callbacks in bridge 
drivers, where you might have an overview of your components.

Otherwise, I can happily carry this in soc-camera for now, I'd just 
request to officially reserve vdev->dev.type for these purposes.

> But if you have ideas to improve the core to make it easier to add pm
> support to the drivers that need it, then I am all for it.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [PATCH/RESEND] soc-camera: add runtime pm support for subdevices

[Mauro Carvalho Chehab]

Hans Verkuil wrote:
>> On Tue, 9 Feb 2010, Hans Verkuil wrote:
>>
>>>> On Mon, 8 Feb 2010, Mauro Carvalho Chehab wrote:
>>>>
>>>>> In fact, on all drivers, there are devices that needs to be turn on
>>> only
>>>>> when
>>>>> streaming is happening: sensors, analog TV/audio demods, digital
>>> demods.
>>>>> Also,
>>>>> a few devices (for example: TV tuners) could eventually be on power
>>> off
>>>>> when
>>>>> no device is opened.
>>>>>
>>>>> As the V4L core knows when this is happening (due to
>>>>> open/close/poll/streamon/reqbuf/qbuf/dqbuf hooks, I think the runtime
>>>>> management
>>>>> can happen at V4L core level.
>>>> Well, we can move it up to v4l core. Should it get any more
>>> complicated
>>>> than adding
>>>>
>>>> 	ret = pm_runtime_resume(&vdev->dev);
>>>> 	if (ret < 0 && ret != -ENOSYS)
>>>> 		return ret;
>>>>
>>>> to v4l2_open() and
>>>>
>>>> 	pm_runtime_suspend(&vdev->dev);
>>>>
>>>> to v4l2_release()?
>>> My apologies if I say something stupid as I know little about pm: are
>>> you
>>> assuming here that streaming only happens on one device node? That may
>>> be
>>> true for soc-camera, but other devices can have multiple streaming nodes
>>> (video, vbi, mpeg, etc). So the call to v4l2_release does not
>>> necessarily
>>> mean that streaming has stopped.
>> Of course you're right, and it concerns not only multiple streaming modes,
>> but simple cases of multiple openings of one node. I was too fast to
>> transfer the implementation from soc-camera to v4l2 - in soc-camera I'm
>> counting opens and closes and only calling pm hooks on first open and last
>> close. So, if we want to put it in v4l-core, we'd have to do something
>> similar, I presume.
> 
> I wouldn't mind having such counters. There are more situations where
> knowing whether it is the first open or last close comes in handy.

A simple count for an open bind to one device node is not enough, since you
may have just one open for /dev/radio and just one open for /dev/video, but
if /dev/radio is closed, it doesn't mean that you can safely power down it,
as /dev/video is still open.

Yet, I think it is doable.

> However, in general I think that pm shouldn't be done in the core. It is
> just too hardware dependent. E.g. there may both capture and display video
> nodes in the driver. And when the last capture stops you can for example
> power down the receiver chips. The same with display and transmitter
> chips. But if both are controlled by the same driver, then a general open
> counter will not work either.
> 
> But if you have ideas to improve the core to make it easier to add pm
> support to the drivers that need it, then I am all for it.

IMO, the runtime pm should be supported at V4L core, but some callbacks are
needed. Also, I can see some classes of PM at the core:

	TV standard demod and sensors only need to be powerup when streaming.
So, if someone is just opening the device to set some configuration, the config
may be stored on a shadow register but the real device may keep on powerdown state.

	On the other hand, the TV tuner may be needed if someone is, for example,
scanning the channels. Depending on the device, the other components like tv and 
audio demod may be in powerdown state.

So, I think that we'll need some callbacks to the drivers, in order to do the
power management on the applicable components. The final action should be at
the driver level, but supported by the core.

> 
> Regards,
> 
>         Hans
> 
>> Thanks
>> Guennadi
>> ---
>> Guennadi Liakhovetski, Ph.D.
>> Freelance Open-Source Software Developer
>> http://www.open-technology.de/
>>
> 
> 


-- 

Cheers,
Mauro

Re: [PATCH/RESEND] soc-camera: add runtime pm support for subdevices

[Hans Verkuil]

> Hans Verkuil wrote:
>>> On Tue, 9 Feb 2010, Hans Verkuil wrote:
>>>
>>>>> On Mon, 8 Feb 2010, Mauro Carvalho Chehab wrote:
>>>>>
>>>>>> In fact, on all drivers, there are devices that needs to be turn on
>>>> only
>>>>>> when
>>>>>> streaming is happening: sensors, analog TV/audio demods, digital
>>>> demods.
>>>>>> Also,
>>>>>> a few devices (for example: TV tuners) could eventually be on power
>>>> off
>>>>>> when
>>>>>> no device is opened.
>>>>>>
>>>>>> As the V4L core knows when this is happening (due to
>>>>>> open/close/poll/streamon/reqbuf/qbuf/dqbuf hooks, I think the
>>>>>> runtime
>>>>>> management
>>>>>> can happen at V4L core level.
>>>>> Well, we can move it up to v4l core. Should it get any more
>>>> complicated
>>>>> than adding
>>>>>
>>>>> 	ret = pm_runtime_resume(&vdev->dev);
>>>>> 	if (ret < 0 && ret != -ENOSYS)
>>>>> 		return ret;
>>>>>
>>>>> to v4l2_open() and
>>>>>
>>>>> 	pm_runtime_suspend(&vdev->dev);
>>>>>
>>>>> to v4l2_release()?
>>>> My apologies if I say something stupid as I know little about pm: are
>>>> you
>>>> assuming here that streaming only happens on one device node? That may
>>>> be
>>>> true for soc-camera, but other devices can have multiple streaming
>>>> nodes
>>>> (video, vbi, mpeg, etc). So the call to v4l2_release does not
>>>> necessarily
>>>> mean that streaming has stopped.
>>> Of course you're right, and it concerns not only multiple streaming
>>> modes,
>>> but simple cases of multiple openings of one node. I was too fast to
>>> transfer the implementation from soc-camera to v4l2 - in soc-camera I'm
>>> counting opens and closes and only calling pm hooks on first open and
>>> last
>>> close. So, if we want to put it in v4l-core, we'd have to do something
>>> similar, I presume.
>>
>> I wouldn't mind having such counters. There are more situations where
>> knowing whether it is the first open or last close comes in handy.
>
> A simple count for an open bind to one device node is not enough, since
> you
> may have just one open for /dev/radio and just one open for /dev/video,
> but
> if /dev/radio is closed, it doesn't mean that you can safely power down
> it,
> as /dev/video is still open.

Or you can have multiple video nodes capturing (e.g. raw video and MPEG at
the same time as ivtv can do). Or no video devices open at all, but a
framebuffer device is using the display.

> Yet, I think it is doable.
>
>> However, in general I think that pm shouldn't be done in the core. It is
>> just too hardware dependent. E.g. there may both capture and display
>> video
>> nodes in the driver. And when the last capture stops you can for example
>> power down the receiver chips. The same with display and transmitter
>> chips. But if both are controlled by the same driver, then a general
>> open
>> counter will not work either.
>>
>> But if you have ideas to improve the core to make it easier to add pm
>> support to the drivers that need it, then I am all for it.
>
> IMO, the runtime pm should be supported at V4L core, but some callbacks
> are
> needed. Also, I can see some classes of PM at the core:
>
> 	TV standard demod and sensors only need to be powerup when streaming.

Definitely not the demod: that's generally used to detect whether there is
a TV signal and what audio format is used. You want that also when not
streaming. I guess it can be powered down though when no files are open.

> So, if someone is just opening the device to set some configuration, the
> config
> may be stored on a shadow register but the real device may keep on
> powerdown state.
>
> 	On the other hand, the TV tuner may be needed if someone is, for example,
> scanning the channels. Depending on the device, the other components like
> tv and
> audio demod may be in powerdown state.
>
> So, I think that we'll need some callbacks to the drivers, in order to do
> the
> power management on the applicable components. The final action should be
> at
> the driver level, but supported by the core.

I guess the essential information is:

1) is someone using the driver (i.e. is a device node open, which is not
necessarily limited to v4l2-type device nodes)?
2) are we actively streaming from or to some particular input or output?

And we probably need some easy way to detect and set the powersaving state
for each component (subdev or the main v4l2_device).

I really need to research the pm stuff...

Regards,

         Hans

>
>>
>> Regards,
>>
>>         Hans
>>
>>> Thanks
>>> Guennadi
>>> ---
>>> Guennadi Liakhovetski, Ph.D.
>>> Freelance Open-Source Software Developer
>>> http://www.open-technology.de/
>>>
>>
>>
>
>
> --
>
> Cheers,
> Mauro
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>


-- 
Hans Verkuil - video4linux developer - sponsored by TANDBERG Telecom

Re: [PATCH/RESEND] soc-camera: add runtime pm support for subdevices

[Mauro Carvalho Chehab]

Hans Verkuil wrote:
>> Hans Verkuil wrote:

>>> But if you have ideas to improve the core to make it easier to add pm
>>> support to the drivers that need it, then I am all for it.
>> IMO, the runtime pm should be supported at V4L core, but some callbacks
>> are
>> needed. Also, I can see some classes of PM at the core:
>>
>> 	TV standard demod and sensors only need to be powerup when streaming.
> 
> Definitely not the demod: that's generally used to detect whether there is
> a TV signal and what audio format is used. You want that also when not
> streaming. I guess it can be powered down though when no files are open.

This is device-specific: on some devices, the tuner provides the info (and
even decode the audio carrier). Also, on some modes (e. g. radio), the demod 
may be turned off.

>> So, I think that we'll need some callbacks to the drivers, in order to do
>> the
>> power management on the applicable components. The final action should be
>> at
>> the driver level, but supported by the core.
> 
> I guess the essential information is:
> 
> 1) is someone using the driver (i.e. is a device node open, which is not
> necessarily limited to v4l2-type device nodes)?
> 2) are we actively streaming from or to some particular input or output?
> 
> And we probably need some easy way to detect and set the powersaving state
> for each component (subdev or the main v4l2_device).

krefs can be a good alternative to check device usage and enable powersaving,
but we'll need some callbacks to save/restore chip register values per dev/subdev.
> 
> I really need to research the pm stuff...
> 
> Regards,
> 
>          Hans
> 
>>> Regards,
>>>
>>>         Hans
>>>
>>>> Thanks
>>>> Guennadi
>>>> ---
>>>> Guennadi Liakhovetski, Ph.D.
>>>> Freelance Open-Source Software Developer
>>>> http://www.open-technology.de/
>>>>
>>>
>>
>> --
>>
>> Cheers,
>> Mauro
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-media" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
> 
> 


-- 

Cheers,
Mauro

Re: [linux-pm] [PATCH/RESEND] soc-camera: add runtime pm support for subdevices

[Alan Stern]

On Mon, 8 Feb 2010, Guennadi Liakhovetski wrote:

> To save power soc-camera powers subdevices down, when they are not in use, 
> if this is supported by the platform. However, the V4L standard dictates, 
> that video nodes shall preserve configuration between uses. This requires 
> runtime power management, which is implemented by this patch. It allows 
> subdevice drivers to specify their runtime power-management methods, by 
> assigning a type to the video device.
> 
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> ---
> 
> I've posted this patch to linux-media earlier, but I'd also like to get 
> comments on linux-pm, sorry to linux-media falks for a duplicate. To 
> explain a bit - soc_camera.c is a management module, that binds video 
> interfaces on SoCs and sensor drivers. The calls, that I am adding to 
> soc_camera.c shall save and restore sensor registers before they are 
> powered down and after they are powered up.

This patch is not correct as it stands.  If you use runtime PM then the 
system PM resume method has to be changed.  See the discussion in 
section 6 of Documentation/power/runtime_pm.txt.

Alan Stern

Re: [linux-pm] [PATCH/RESEND] soc-camera: add runtime pm support for subdevices

[Guennadi Liakhovetski]

On Tue, 9 Feb 2010, Alan Stern wrote:

> On Mon, 8 Feb 2010, Guennadi Liakhovetski wrote:
> 
> > To save power soc-camera powers subdevices down, when they are not in use, 
> > if this is supported by the platform. However, the V4L standard dictates, 
> > that video nodes shall preserve configuration between uses. This requires 
> > runtime power management, which is implemented by this patch. It allows 
> > subdevice drivers to specify their runtime power-management methods, by 
> > assigning a type to the video device.
> > 
> > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > ---
> > 
> > I've posted this patch to linux-media earlier, but I'd also like to get 
> > comments on linux-pm, sorry to linux-media falks for a duplicate. To 
> > explain a bit - soc_camera.c is a management module, that binds video 
> > interfaces on SoCs and sensor drivers. The calls, that I am adding to 
> > soc_camera.c shall save and restore sensor registers before they are 
> > powered down and after they are powered up.
> 
> This patch is not correct as it stands.  If you use runtime PM then the 
> system PM resume method has to be changed.  See the discussion in 
> section 6 of Documentation/power/runtime_pm.txt.

...changed - for the same device, right? And I don't see any system PM 
methods implemented for these devices yet.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [PATCH/RESEND] soc-camera: add runtime pm support for subdevices

[Guennadi Liakhovetski]

On Tue, 9 Feb 2010, Hans Verkuil wrote:

> 1) is someone using the driver (i.e. is a device node open, which is not
> necessarily limited to v4l2-type device nodes)?
> 2) are we actively streaming from or to some particular input or output?
> 
> And we probably need some easy way to detect and set the powersaving state
> for each component (subdev or the main v4l2_device).
> 
> I really need to research the pm stuff...

That certainly wouldn't hurt, but I have to resolve a problem of a failing 
device now. And I have 3 options:

1. implement runtime_pm in soc-camera and down, using vdev->dev.type. Easy 
and relatively painless - we already have use-counts there, so, first open 
and last close are readily available.

2. add the same as above to v4l2-dev.c - wouldn't really want to do that, 
implications too large, no use-counting.

3. accept the patch from Val and just write registers, we are currently 
losing, on each s_fmt. Easy, but not very efficient - there can be other 
similar problems elsewhere...

Which of them shall I do for now? Besides, I realise, that decisions which 
devices and when to suspend and to wake up are difficult, so, PM requires 
careful implementation. But doesn't this also mean, that we anyway cannot 
implement any specifics in the top level, and can only implement hooks to 
call hardware-specific pm callbacks, so, probably, the bridge driver will 
have to implement specifics anyway?

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [linux-pm] [PATCH/RESEND] soc-camera: add runtime pm support for subdevices

[Alan Stern]

On Tue, 9 Feb 2010, Guennadi Liakhovetski wrote:

> On Tue, 9 Feb 2010, Alan Stern wrote:
> 
> > On Mon, 8 Feb 2010, Guennadi Liakhovetski wrote:
> > 
> > > To save power soc-camera powers subdevices down, when they are not in use, 
> > > if this is supported by the platform. However, the V4L standard dictates, 
> > > that video nodes shall preserve configuration between uses. This requires 
> > > runtime power management, which is implemented by this patch. It allows 
> > > subdevice drivers to specify their runtime power-management methods, by 
> > > assigning a type to the video device.
> > > 
> > > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > > ---
> > > 
> > > I've posted this patch to linux-media earlier, but I'd also like to get 
> > > comments on linux-pm, sorry to linux-media falks for a duplicate. To 
> > > explain a bit - soc_camera.c is a management module, that binds video 
> > > interfaces on SoCs and sensor drivers. The calls, that I am adding to 
> > > soc_camera.c shall save and restore sensor registers before they are 
> > > powered down and after they are powered up.
> > 
> > This patch is not correct as it stands.  If you use runtime PM then the 
> > system PM resume method has to be changed.  See the discussion in 
> > section 6 of Documentation/power/runtime_pm.txt.
> 
> ...changed - for the same device, right? And I don't see any system PM 
> methods implemented for these devices yet.

Oh, if these new routines are just bus glue then you're right, it 
doesn't matter.  The changes would have to be made in the lower-level 
drivers, when they implement runtime PM.

Alan Stern