linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC] subdevice PM: .s_power() deprecation?
@ 2011-10-03 10:57 Guennadi Liakhovetski
  2011-10-08 21:36 ` Sakari Ailus
  0 siblings, 1 reply; 21+ messages in thread
From: Guennadi Liakhovetski @ 2011-10-03 10:57 UTC (permalink / raw)
  To: Linux Media Mailing List; +Cc: Laurent Pinchart

Hi all

(The original .s_power() author added to cc;-))

Here comes one more Request for Discussion from me.

Short: on what events, at which level and how shall subdevice PM be 
envoked?

Subdevices can have varying and arbitrarily complex Power Management 
methods. On-SoC subdevices would typically be powered on and off by 
writing to some system registers. External subdevices (sensors etc.) can 
be powered on or off by something as simple as a GPIO, or can use several 
power regulators, supplying power to different device circuits. This 
means, a part of this knowledge belongs directly to the driver, while 
another part of it comes from platform data. The driver itself knows, 
whether it can control device's power, using internal capabilities, or it 
has to request a certain number of regulators. In the latter case, 
perhaps, it would be sane to assume, that if a certain regulator is not 
available, then the respective voltage is supplied by the system 
statically.

When to invoke? Subdeices can be used in two cases: for configuration and 
for data processing (streaming). For configuration the driver can choose 
one of two approaches: (1) cache all configuration requests and only 
execute them on STREAMON. Advantages: (a) the device can be kept off all 
the time during configuration, (b) the order is unimportant: the driver 
only stores values and applies them in the "correct" order. Disadvantages: 
(a) if the result of any such operation cannot be fully predicted by the 
driver, it cannot be reported to the user immediately after the operation 
execution but only at the STREAMON time (does anyone know any such 
"volatile" operations?), (b) the order is lost (is this important?). (2) 
execute all operations immediately. Advantages and disadvantages: just 
invert those from (1) above.

So far individual drivers decide themselves which way to go. This way only 
drivers themselves know, when and what parts of the device they have to 
power on and off for configuration. The only thing the bridge driver can 
be sure about is, that all the involved subdevices in the pipeline have to 
be powered on during streaming. But even then - maybe the driver can and 
wants to power the i2c circuitry off for that time?

All the above makes me think, that .s_power() methods are actually useless 
in the "operation context." The bridge has basically no way to know, when 
and which parts of the subdevice to power on or off. Subdevice 
configuration is anyway always performed, using the driver, and for 
streaming all participating subdevices just have to be informed about 
streaming begin and end.

The only pure PM activity, that subdevice drivers have to be informed 
about are suspends and resumes. Normal bus PM callbacks are not always 
usable in our case. E.g., you cannot use i2c PM, because i2c can well be 
resumed before the bridge and then camera sensors typically still cannot 
be accessed over i2c.

Therefore I propose to either deprecate (and later remove) .s_power() and 
add .suspend() and .resume() instead or repurpose .s_power() to be _only_ 
used for system-wide suspending and resuming. Even for runtime PM the 
subdevice driver has all the chances to decide itself when and how to save 
power, so, again, there is no need to be called from outside.

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC] subdevice PM: .s_power() deprecation?
  2011-10-03 10:57 [RFC] subdevice PM: .s_power() deprecation? Guennadi Liakhovetski
@ 2011-10-08 21:36 ` Sakari Ailus
  2011-10-17  8:06   ` Guennadi Liakhovetski
  0 siblings, 1 reply; 21+ messages in thread
From: Sakari Ailus @ 2011-10-08 21:36 UTC (permalink / raw)
  To: Guennadi Liakhovetski; +Cc: Linux Media Mailing List, Laurent Pinchart

On Mon, Oct 03, 2011 at 12:57:10PM +0200, Guennadi Liakhovetski wrote:
> Hi all

Hi Guennadi,

Thanks for a thoughtful writing on subdev PM!

> (The original .s_power() author added to cc;-))
> 
> Here comes one more Request for Discussion from me.
> 
> Short: on what events, at which level and how shall subdevice PM be 
> envoked?
> 
> Subdevices can have varying and arbitrarily complex Power Management 
> methods. On-SoC subdevices would typically be powered on and off by 
> writing to some system registers. External subdevices (sensors etc.) can 
> be powered on or off by something as simple as a GPIO, or can use several 
> power regulators, supplying power to different device circuits. This 
> means, a part of this knowledge belongs directly to the driver, while 
> another part of it comes from platform data. The driver itself knows, 
> whether it can control device's power, using internal capabilities, or it 
> has to request a certain number of regulators. In the latter case, 
> perhaps, it would be sane to assume, that if a certain regulator is not 
> available, then the respective voltage is supplied by the system 
> statically.
> 
> When to invoke? Subdeices can be used in two cases: for configuration and 
> for data processing (streaming). For configuration the driver can choose 
> one of two approaches: (1) cache all configuration requests and only 
> execute them on STREAMON. Advantages: (a) the device can be kept off all 
> the time during configuration, (b) the order is unimportant: the driver 
> only stores values and applies them in the "correct" order. Disadvantages: 
> (a) if the result of any such operation cannot be fully predicted by the 
> driver, it cannot be reported to the user immediately after the operation 
> execution but only at the STREAMON time (does anyone know any such 
> "volatile" operations?), (b) the order is lost (is this important?). (2) 
> execute all operations immediately. Advantages and disadvantages: just 
> invert those from (1) above.
> 
> So far individual drivers decide themselves which way to go. This way only 
> drivers themselves know, when and what parts of the device they have to 
> power on and off for configuration. The only thing the bridge driver can 
> be sure about is, that all the involved subdevices in the pipeline have to 
> be powered on during streaming. But even then - maybe the driver can and 
> wants to power the i2c circuitry off for that time?

The bridge driver can't (nor should) know about the power management
requirements of random subdevs. The name of the s_power op is rather
poitless in its current state.

The power state of the subdev probably even never matters to the bridge ---
or do we really have an example of that?

In my opinion the bridge driver should instead tell the bridge drivers what
they can expect to hear from the bridge --- for example that the bridge can
issue set / get controls or fmt ops to the subdev. The subdev may or may not
need to be powered for those: only the subdev driver knows.

This is analogous to opening the subdev node from user space. Anything else
except streaming is allowed. And streaming, which for sure requires powering
on the subdev, is already nicely handled by the s_stream op.

What do you think?

In practice the name of s_power should change, as well as possible
implementatio on subdev drivers.

> All the above makes me think, that .s_power() methods are actually useless 
> in the "operation context." The bridge has basically no way to know, when 
> and which parts of the subdevice to power on or off. Subdevice 
> configuration is anyway always performed, using the driver, and for 
> streaming all participating subdevices just have to be informed about 
> streaming begin and end.
> 
> The only pure PM activity, that subdevice drivers have to be informed 
> about are suspends and resumes. Normal bus PM callbacks are not always 
> usable in our case. E.g., you cannot use i2c PM, because i2c can well be 
> resumed before the bridge and then camera sensors typically still cannot 
> be accessed over i2c.

Do you have a bridge that provides a clock to subdevs? The clock should be
modelled in the clock framework --- yes, I guess there's still a way to go
before that,s universally possible.

> Therefore I propose to either deprecate (and later remove) .s_power() and 
> add .suspend() and .resume() instead or repurpose .s_power() to be _only_ 
> used for system-wide suspending and resuming. Even for runtime PM the 
> subdevice driver has all the chances to decide itself when and how to save 
> power, so, again, there is no need to be called from outside.

-- 
Sakari Ailus
e-mail: sakari.ailus@iki.fi	jabber/XMPP/Gmail: sailus@retiisi.org.uk

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC] subdevice PM: .s_power() deprecation?
  2011-10-08 21:36 ` Sakari Ailus
@ 2011-10-17  8:06   ` Guennadi Liakhovetski
  2011-10-17 12:12     ` Laurent Pinchart
  2011-10-17 12:59     ` Sylwester Nawrocki
  0 siblings, 2 replies; 21+ messages in thread
From: Guennadi Liakhovetski @ 2011-10-17  8:06 UTC (permalink / raw)
  To: Sakari Ailus; +Cc: Linux Media Mailing List, Laurent Pinchart

Hi Sakari

On Sun, 9 Oct 2011, Sakari Ailus wrote:

> On Mon, Oct 03, 2011 at 12:57:10PM +0200, Guennadi Liakhovetski wrote:
> > Hi all
> 
> Hi Guennadi,
> 
> Thanks for a thoughtful writing on subdev PM!
> 
> > (The original .s_power() author added to cc;-))
> > 
> > Here comes one more Request for Discussion from me.
> > 
> > Short: on what events, at which level and how shall subdevice PM be 
> > envoked?
> > 
> > Subdevices can have varying and arbitrarily complex Power Management 
> > methods. On-SoC subdevices would typically be powered on and off by 
> > writing to some system registers. External subdevices (sensors etc.) can 
> > be powered on or off by something as simple as a GPIO, or can use several 
> > power regulators, supplying power to different device circuits. This 
> > means, a part of this knowledge belongs directly to the driver, while 
> > another part of it comes from platform data. The driver itself knows, 
> > whether it can control device's power, using internal capabilities, or it 
> > has to request a certain number of regulators. In the latter case, 
> > perhaps, it would be sane to assume, that if a certain regulator is not 
> > available, then the respective voltage is supplied by the system 
> > statically.
> > 
> > When to invoke? Subdeices can be used in two cases: for configuration and 
> > for data processing (streaming). For configuration the driver can choose 
> > one of two approaches: (1) cache all configuration requests and only 
> > execute them on STREAMON. Advantages: (a) the device can be kept off all 
> > the time during configuration, (b) the order is unimportant: the driver 
> > only stores values and applies them in the "correct" order. Disadvantages: 
> > (a) if the result of any such operation cannot be fully predicted by the 
> > driver, it cannot be reported to the user immediately after the operation 
> > execution but only at the STREAMON time (does anyone know any such 
> > "volatile" operations?), (b) the order is lost (is this important?). (2) 
> > execute all operations immediately. Advantages and disadvantages: just 
> > invert those from (1) above.
> > 
> > So far individual drivers decide themselves which way to go. This way only 
> > drivers themselves know, when and what parts of the device they have to 
> > power on and off for configuration. The only thing the bridge driver can 
> > be sure about is, that all the involved subdevices in the pipeline have to 
> > be powered on during streaming. But even then - maybe the driver can and 
> > wants to power the i2c circuitry off for that time?
> 
> The bridge driver can't (nor should) know about the power management
> requirements of random subdevs. The name of the s_power op is rather
> poitless in its current state.
> 
> The power state of the subdev probably even never matters to the bridge ---

Exactly, that's the conclusion I come to in this RFC too.

> or do we really have an example of that?
> 
> In my opinion the bridge driver should instead tell the bridge drivers what
> they can expect to hear from the bridge --- for example that the bridge can
> issue set / get controls or fmt ops to the subdev. The subdev may or may not
> need to be powered for those: only the subdev driver knows.

Hm, why should the bridge driver tell the subdev driver (I presume, that's 
a typo in your above sentence) what to _expect_? Isn't just calling those 
operations enough?

> This is analogous to opening the subdev node from user space. Anything else
> except streaming is allowed. And streaming, which for sure requires powering
> on the subdev, is already nicely handled by the s_stream op.
> 
> What do you think?
> 
> In practice the name of s_power should change, as well as possible
> implementatio on subdev drivers.

But why do we need it at all?

> > All the above makes me think, that .s_power() methods are actually useless 
> > in the "operation context." The bridge has basically no way to know, when 
> > and which parts of the subdevice to power on or off. Subdevice 
> > configuration is anyway always performed, using the driver, and for 
> > streaming all participating subdevices just have to be informed about 
> > streaming begin and end.
> > 
> > The only pure PM activity, that subdevice drivers have to be informed 
> > about are suspends and resumes. Normal bus PM callbacks are not always 
> > usable in our case. E.g., you cannot use i2c PM, because i2c can well be 
> > resumed before the bridge and then camera sensors typically still cannot 
> > be accessed over i2c.
> 
> Do you have a bridge that provides a clock to subdevs? The clock should be
> modelled in the clock framework --- yes, I guess there's still a way to go
> before that,s universally possible.

sure, almost all cameras in my configurations get their master clock from 
the bridge - usually a dedicated camera master clock output.

> > Therefore I propose to either deprecate (and later remove) .s_power() and 
> > add .suspend() and .resume() instead or repurpose .s_power() to be _only_ 
> > used for system-wide suspending and resuming. Even for runtime PM the 
> > subdevice driver has all the chances to decide itself when and how to save 
> > power, so, again, there is no need to be called from outside.

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC] subdevice PM: .s_power() deprecation?
  2011-10-17  8:06   ` Guennadi Liakhovetski
@ 2011-10-17 12:12     ` Laurent Pinchart
  2011-10-17 12:37       ` Guennadi Liakhovetski
  2011-10-17 12:59     ` Sylwester Nawrocki
  1 sibling, 1 reply; 21+ messages in thread
From: Laurent Pinchart @ 2011-10-17 12:12 UTC (permalink / raw)
  To: Guennadi Liakhovetski; +Cc: Sakari Ailus, Linux Media Mailing List

Hi Guennadi,

On Monday 17 October 2011 10:06:53 Guennadi Liakhovetski wrote:
> On Sun, 9 Oct 2011, Sakari Ailus wrote:
> > On Mon, Oct 03, 2011 at 12:57:10PM +0200, Guennadi Liakhovetski wrote:
> > > Hi all
> > 
> > Hi Guennadi,
> > 
> > Thanks for a thoughtful writing on subdev PM!
> > 
> > > (The original .s_power() author added to cc;-))
> > > 
> > > Here comes one more Request for Discussion from me.
> > > 
> > > Short: on what events, at which level and how shall subdevice PM be
> > > envoked?
> > > 
> > > Subdevices can have varying and arbitrarily complex Power Management
> > > methods. On-SoC subdevices would typically be powered on and off by
> > > writing to some system registers. External subdevices (sensors etc.)
> > > can be powered on or off by something as simple as a GPIO, or can use
> > > several power regulators, supplying power to different device
> > > circuits. This means, a part of this knowledge belongs directly to the
> > > driver, while another part of it comes from platform data. The driver
> > > itself knows, whether it can control device's power, using internal
> > > capabilities, or it has to request a certain number of regulators. In
> > > the latter case, perhaps, it would be sane to assume, that if a
> > > certain regulator is not available, then the respective voltage is
> > > supplied by the system statically.
> > > 
> > > When to invoke? Subdeices can be used in two cases: for configuration
> > > and for data processing (streaming). For configuration the driver can
> > > choose one of two approaches: (1) cache all configuration requests and
> > > only execute them on STREAMON. Advantages: (a) the device can be kept
> > > off all the time during configuration, (b) the order is unimportant:
> > > the driver only stores values and applies them in the "correct" order.
> > > Disadvantages: (a) if the result of any such operation cannot be fully
> > > predicted by the driver, it cannot be reported to the user immediately
> > > after the operation execution but only at the STREAMON time (does
> > > anyone know any such "volatile" operations?), (b) the order is lost
> > > (is this important?). (2) execute all operations immediately.
> > > Advantages and disadvantages: just invert those from (1) above.
> > > 
> > > So far individual drivers decide themselves which way to go. This way
> > > only drivers themselves know, when and what parts of the device they
> > > have to power on and off for configuration. The only thing the bridge
> > > driver can be sure about is, that all the involved subdevices in the
> > > pipeline have to be powered on during streaming. But even then - maybe
> > > the driver can and wants to power the i2c circuitry off for that time?
> > 
> > The bridge driver can't (nor should) know about the power management
> > requirements of random subdevs. The name of the s_power op is rather
> > poitless in its current state.
> > 
> > The power state of the subdev probably even never matters to the bridge
> > ---
> 
> Exactly, that's the conclusion I come to in this RFC too.
> 
> > or do we really have an example of that?
> > 
> > In my opinion the bridge driver should instead tell the bridge drivers
> > what they can expect to hear from the bridge --- for example that the
> > bridge can issue set / get controls or fmt ops to the subdev. The subdev
> > may or may not need to be powered for those: only the subdev driver
> > knows.
> 
> Hm, why should the bridge driver tell the subdev driver (I presume, that's
> a typo in your above sentence) what to _expect_? Isn't just calling those
> operations enough?
> 
> > This is analogous to opening the subdev node from user space. Anything
> > else except streaming is allowed. And streaming, which for sure requires
> > powering on the subdev, is already nicely handled by the s_stream op.
> > 
> > What do you think?
> > 
> > In practice the name of s_power should change, as well as possible
> > implementatio on subdev drivers.
> 
> But why do we need it at all?

PM QoS ? Waking up a sensor can introduce long latencies, and you might want 
to do this before starting the stream depending on your use cases. This will 
likely not be solved by .s_power() alone though.

> > > All the above makes me think, that .s_power() methods are actually
> > > useless in the "operation context." The bridge has basically no way to
> > > know, when and which parts of the subdevice to power on or off.
> > > Subdevice configuration is anyway always performed, using the driver,
> > > and for streaming all participating subdevices just have to be
> > > informed about streaming begin and end.
> > > 
> > > The only pure PM activity, that subdevice drivers have to be informed
> > > about are suspends and resumes. Normal bus PM callbacks are not always
> > > usable in our case. E.g., you cannot use i2c PM, because i2c can well
> > > be resumed before the bridge and then camera sensors typically still
> > > cannot be accessed over i2c.
> > 
> > Do you have a bridge that provides a clock to subdevs? The clock should
> > be modelled in the clock framework --- yes, I guess there's still a way
> > to go before that,s universally possible.
> 
> sure, almost all cameras in my configurations get their master clock from
> the bridge - usually a dedicated camera master clock output.
> 
> > > Therefore I propose to either deprecate (and later remove) .s_power()
> > > and add .suspend() and .resume() instead or repurpose .s_power() to be
> > > _only_ used for system-wide suspending and resuming. Even for runtime
> > > PM the subdevice driver has all the chances to decide itself when and
> > > how to save power, so, again, there is no need to be called from
> > > outside.

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC] subdevice PM: .s_power() deprecation?
  2011-10-17 12:12     ` Laurent Pinchart
@ 2011-10-17 12:37       ` Guennadi Liakhovetski
  0 siblings, 0 replies; 21+ messages in thread
From: Guennadi Liakhovetski @ 2011-10-17 12:37 UTC (permalink / raw)
  To: Laurent Pinchart; +Cc: Sakari Ailus, Linux Media Mailing List

Hi Laurent

On Mon, 17 Oct 2011, Laurent Pinchart wrote:

> Hi Guennadi,
> 
> On Monday 17 October 2011 10:06:53 Guennadi Liakhovetski wrote:
> > On Sun, 9 Oct 2011, Sakari Ailus wrote:
> > > On Mon, Oct 03, 2011 at 12:57:10PM +0200, Guennadi Liakhovetski wrote:
> > > > Hi all
> > > 
> > > Hi Guennadi,
> > > 
> > > Thanks for a thoughtful writing on subdev PM!
> > > 
> > > > (The original .s_power() author added to cc;-))
> > > > 
> > > > Here comes one more Request for Discussion from me.
> > > > 
> > > > Short: on what events, at which level and how shall subdevice PM be
> > > > envoked?
> > > > 
> > > > Subdevices can have varying and arbitrarily complex Power Management
> > > > methods. On-SoC subdevices would typically be powered on and off by
> > > > writing to some system registers. External subdevices (sensors etc.)
> > > > can be powered on or off by something as simple as a GPIO, or can use
> > > > several power regulators, supplying power to different device
> > > > circuits. This means, a part of this knowledge belongs directly to the
> > > > driver, while another part of it comes from platform data. The driver
> > > > itself knows, whether it can control device's power, using internal
> > > > capabilities, or it has to request a certain number of regulators. In
> > > > the latter case, perhaps, it would be sane to assume, that if a
> > > > certain regulator is not available, then the respective voltage is
> > > > supplied by the system statically.
> > > > 
> > > > When to invoke? Subdeices can be used in two cases: for configuration
> > > > and for data processing (streaming). For configuration the driver can
> > > > choose one of two approaches: (1) cache all configuration requests and
> > > > only execute them on STREAMON. Advantages: (a) the device can be kept
> > > > off all the time during configuration, (b) the order is unimportant:
> > > > the driver only stores values and applies them in the "correct" order.
> > > > Disadvantages: (a) if the result of any such operation cannot be fully
> > > > predicted by the driver, it cannot be reported to the user immediately
> > > > after the operation execution but only at the STREAMON time (does
> > > > anyone know any such "volatile" operations?), (b) the order is lost
> > > > (is this important?). (2) execute all operations immediately.
> > > > Advantages and disadvantages: just invert those from (1) above.
> > > > 
> > > > So far individual drivers decide themselves which way to go. This way
> > > > only drivers themselves know, when and what parts of the device they
> > > > have to power on and off for configuration. The only thing the bridge
> > > > driver can be sure about is, that all the involved subdevices in the
> > > > pipeline have to be powered on during streaming. But even then - maybe
> > > > the driver can and wants to power the i2c circuitry off for that time?
> > > 
> > > The bridge driver can't (nor should) know about the power management
> > > requirements of random subdevs. The name of the s_power op is rather
> > > poitless in its current state.
> > > 
> > > The power state of the subdev probably even never matters to the bridge
> > > ---
> > 
> > Exactly, that's the conclusion I come to in this RFC too.
> > 
> > > or do we really have an example of that?
> > > 
> > > In my opinion the bridge driver should instead tell the bridge drivers
> > > what they can expect to hear from the bridge --- for example that the
> > > bridge can issue set / get controls or fmt ops to the subdev. The subdev
> > > may or may not need to be powered for those: only the subdev driver
> > > knows.
> > 
> > Hm, why should the bridge driver tell the subdev driver (I presume, that's
> > a typo in your above sentence) what to _expect_? Isn't just calling those
> > operations enough?
> > 
> > > This is analogous to opening the subdev node from user space. Anything
> > > else except streaming is allowed. And streaming, which for sure requires
> > > powering on the subdev, is already nicely handled by the s_stream op.
> > > 
> > > What do you think?
> > > 
> > > In practice the name of s_power should change, as well as possible
> > > implementatio on subdev drivers.
> > 
> > But why do we need it at all?
> 
> PM QoS ? Waking up a sensor can introduce long latencies, and you might want 
> to do this before starting the stream depending on your use cases. This will 
> likely not be solved by .s_power() alone though.

I'm not yet familiar with PM QoS - will have to look at it very soon, but 
in any case so far this is a purely in-kernel API, right? Which part of 
the kernel currently knows better than the subdev driver itself, when to 
power on? I would understand, if the user had a way to indicate - don't 
power off, I'll be using it again soon. But otherwise?

Thanks
Guennadi

> > > > All the above makes me think, that .s_power() methods are actually
> > > > useless in the "operation context." The bridge has basically no way to
> > > > know, when and which parts of the subdevice to power on or off.
> > > > Subdevice configuration is anyway always performed, using the driver,
> > > > and for streaming all participating subdevices just have to be
> > > > informed about streaming begin and end.
> > > > 
> > > > The only pure PM activity, that subdevice drivers have to be informed
> > > > about are suspends and resumes. Normal bus PM callbacks are not always
> > > > usable in our case. E.g., you cannot use i2c PM, because i2c can well
> > > > be resumed before the bridge and then camera sensors typically still
> > > > cannot be accessed over i2c.
> > > 
> > > Do you have a bridge that provides a clock to subdevs? The clock should
> > > be modelled in the clock framework --- yes, I guess there's still a way
> > > to go before that,s universally possible.
> > 
> > sure, almost all cameras in my configurations get their master clock from
> > the bridge - usually a dedicated camera master clock output.
> > 
> > > > Therefore I propose to either deprecate (and later remove) .s_power()
> > > > and add .suspend() and .resume() instead or repurpose .s_power() to be
> > > > _only_ used for system-wide suspending and resuming. Even for runtime
> > > > PM the subdevice driver has all the chances to decide itself when and
> > > > how to save power, so, again, there is no need to be called from
> > > > outside.
> 
> -- 
> Regards,
> 
> Laurent Pinchart
> 

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC] subdevice PM: .s_power() deprecation?
  2011-10-17  8:06   ` Guennadi Liakhovetski
  2011-10-17 12:12     ` Laurent Pinchart
@ 2011-10-17 12:59     ` Sylwester Nawrocki
  2011-10-17 13:49       ` Guennadi Liakhovetski
  1 sibling, 1 reply; 21+ messages in thread
From: Sylwester Nawrocki @ 2011-10-17 12:59 UTC (permalink / raw)
  To: Guennadi Liakhovetski
  Cc: Sakari Ailus, Linux Media Mailing List, Laurent Pinchart

Hi,

On 10/17/2011 10:06 AM, Guennadi Liakhovetski wrote:
> On Sun, 9 Oct 2011, Sakari Ailus wrote:
>> On Mon, Oct 03, 2011 at 12:57:10PM +0200, Guennadi Liakhovetski wrote:
>> Thanks for a thoughtful writing on subdev PM!
>>
>>> (The original .s_power() author added to cc;-))
>>>
>>> Here comes one more Request for Discussion from me.
>>>
>>> Short: on what events, at which level and how shall subdevice PM be 
>>> envoked?
>>>
>>> Subdevices can have varying and arbitrarily complex Power Management 
>>> methods. On-SoC subdevices would typically be powered on and off by 
>>> writing to some system registers. External subdevices (sensors etc.) can 
>>> be powered on or off by something as simple as a GPIO, or can use several 
>>> power regulators, supplying power to different device circuits. This 
>>> means, a part of this knowledge belongs directly to the driver, while 
>>> another part of it comes from platform data. The driver itself knows, 
>>> whether it can control device's power, using internal capabilities, or it 
>>> has to request a certain number of regulators. In the latter case, 
>>> perhaps, it would be sane to assume, that if a certain regulator is not 
>>> available, then the respective voltage is supplied by the system 
>>> statically.
>>>
>>> When to invoke? Subdeices can be used in two cases: for configuration and 
>>> for data processing (streaming). For configuration the driver can choose 
>>> one of two approaches: (1) cache all configuration requests and only 
>>> execute them on STREAMON. Advantages: (a) the device can be kept off all 
>>> the time during configuration, (b) the order is unimportant: the driver 
>>> only stores values and applies them in the "correct" order. Disadvantages: 
>>> (a) if the result of any such operation cannot be fully predicted by the 
>>> driver, it cannot be reported to the user immediately after the operation 
>>> execution but only at the STREAMON time (does anyone know any such 
>>> "volatile" operations?), (b) the order is lost (is this important?). (2) 
>>> execute all operations immediately. Advantages and disadvantages: just 
>>> invert those from (1) above.
>>>
>>> So far individual drivers decide themselves which way to go. This way only 
>>> drivers themselves know, when and what parts of the device they have to 
>>> power on and off for configuration. The only thing the bridge driver can 
>>> be sure about is, that all the involved subdevices in the pipeline have to 
>>> be powered on during streaming. But even then - maybe the driver can and 
>>> wants to power the i2c circuitry off for that time?
>>
>> The bridge driver can't (nor should) know about the power management
>> requirements of random subdevs. The name of the s_power op is rather
>> poitless in its current state.
>>
>> The power state of the subdev probably even never matters to the bridge ---
> 
> Exactly, that's the conclusion I come to in this RFC too.
> 
>> or do we really have an example of that?
>>
>> In my opinion the bridge driver should instead tell the bridge drivers what
>> they can expect to hear from the bridge --- for example that the bridge can
>> issue set / get controls or fmt ops to the subdev. The subdev may or may not
>> need to be powered for those: only the subdev driver knows.
> 
> Hm, why should the bridge driver tell the subdev driver (I presume, that's 
> a typo in your above sentence) what to _expect_? Isn't just calling those 
> operations enough?
> 
>> This is analogous to opening the subdev node from user space. Anything else
>> except streaming is allowed. And streaming, which for sure requires powering
>> on the subdev, is already nicely handled by the s_stream op.
>>
>> What do you think?
>>
>> In practice the name of s_power should change, as well as possible
>> implementatio on subdev drivers.
> 
> But why do we need it at all?

AFAICS in some TV card drivers it is used to put the analog tuner into low
power state.
So core.s_power op provides the mans to suspend/resume a sub-device.

If the bridge driver only implements a user space interface for the subdev,
it may want to bring a subdev up in some specific moment, before video.s_stream,
e.g. in some ioctl or at device open(), etc.

Let's imagine bringing the sensor up takes appr. 700 ms, often we don't want 
the sensor driver to be doing this before every s_stream().

I agree the subdev drivers have best knowledge as to when to enable/disable
power, from their device point of view. 

But how could we resolve the high latencies issue without something like 
the s_power() callback ?

> 
>>> All the above makes me think, that .s_power() methods are actually useless 
>>> in the "operation context." The bridge has basically no way to know, when 
>>> and which parts of the subdevice to power on or off. Subdevice 
>>> configuration is anyway always performed, using the driver, and for 
>>> streaming all participating subdevices just have to be informed about 
>>> streaming begin and end.
>>>
>>> The only pure PM activity, that subdevice drivers have to be informed 
>>> about are suspends and resumes. Normal bus PM callbacks are not always 
>>> usable in our case. E.g., you cannot use i2c PM, because i2c can well be 
>>> resumed before the bridge and then camera sensors typically still cannot 
>>> be accessed over i2c.
>>
>> Do you have a bridge that provides a clock to subdevs? The clock should be
>> modelled in the clock framework --- yes, I guess there's still a way to go
>> before that,s universally possible.
> 
> sure, almost all cameras in my configurations get their master clock from 
> the bridge - usually a dedicated camera master clock output.
> 
>>> Therefore I propose to either deprecate (and later remove) .s_power() and 
>>> add .suspend() and .resume() instead or repurpose .s_power() to be _only_ 
>>> used for system-wide suspending and resuming. Even for runtime PM the 
>>> subdevice driver has all the chances to decide itself when and how to save 
>>> power, so, again, there is no need to be called from outside.

Although more explicit, two suspend/resume callbacks in place of single s_power
doesn't seem to add much value to me.


Regards,
-- 
Sylwester Nawrocki
Samsung Poland R&D Center

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC] subdevice PM: .s_power() deprecation?
  2011-10-17 12:59     ` Sylwester Nawrocki
@ 2011-10-17 13:49       ` Guennadi Liakhovetski
  2011-10-17 15:15         ` Sylwester Nawrocki
  0 siblings, 1 reply; 21+ messages in thread
From: Guennadi Liakhovetski @ 2011-10-17 13:49 UTC (permalink / raw)
  To: Sylwester Nawrocki
  Cc: Sakari Ailus, Linux Media Mailing List, Laurent Pinchart

On Mon, 17 Oct 2011, Sylwester Nawrocki wrote:

> Hi,
> 
> On 10/17/2011 10:06 AM, Guennadi Liakhovetski wrote:
> > On Sun, 9 Oct 2011, Sakari Ailus wrote:
> >> On Mon, Oct 03, 2011 at 12:57:10PM +0200, Guennadi Liakhovetski wrote:
> >> Thanks for a thoughtful writing on subdev PM!
> >>
> >>> (The original .s_power() author added to cc;-))
> >>>
> >>> Here comes one more Request for Discussion from me.
> >>>
> >>> Short: on what events, at which level and how shall subdevice PM be 
> >>> envoked?
> >>>
> >>> Subdevices can have varying and arbitrarily complex Power Management 
> >>> methods. On-SoC subdevices would typically be powered on and off by 
> >>> writing to some system registers. External subdevices (sensors etc.) can 
> >>> be powered on or off by something as simple as a GPIO, or can use several 
> >>> power regulators, supplying power to different device circuits. This 
> >>> means, a part of this knowledge belongs directly to the driver, while 
> >>> another part of it comes from platform data. The driver itself knows, 
> >>> whether it can control device's power, using internal capabilities, or it 
> >>> has to request a certain number of regulators. In the latter case, 
> >>> perhaps, it would be sane to assume, that if a certain regulator is not 
> >>> available, then the respective voltage is supplied by the system 
> >>> statically.
> >>>
> >>> When to invoke? Subdeices can be used in two cases: for configuration and 
> >>> for data processing (streaming). For configuration the driver can choose 
> >>> one of two approaches: (1) cache all configuration requests and only 
> >>> execute them on STREAMON. Advantages: (a) the device can be kept off all 
> >>> the time during configuration, (b) the order is unimportant: the driver 
> >>> only stores values and applies them in the "correct" order. Disadvantages: 
> >>> (a) if the result of any such operation cannot be fully predicted by the 
> >>> driver, it cannot be reported to the user immediately after the operation 
> >>> execution but only at the STREAMON time (does anyone know any such 
> >>> "volatile" operations?), (b) the order is lost (is this important?). (2) 
> >>> execute all operations immediately. Advantages and disadvantages: just 
> >>> invert those from (1) above.
> >>>
> >>> So far individual drivers decide themselves which way to go. This way only 
> >>> drivers themselves know, when and what parts of the device they have to 
> >>> power on and off for configuration. The only thing the bridge driver can 
> >>> be sure about is, that all the involved subdevices in the pipeline have to 
> >>> be powered on during streaming. But even then - maybe the driver can and 
> >>> wants to power the i2c circuitry off for that time?
> >>
> >> The bridge driver can't (nor should) know about the power management
> >> requirements of random subdevs. The name of the s_power op is rather
> >> poitless in its current state.
> >>
> >> The power state of the subdev probably even never matters to the bridge ---
> > 
> > Exactly, that's the conclusion I come to in this RFC too.
> > 
> >> or do we really have an example of that?
> >>
> >> In my opinion the bridge driver should instead tell the bridge drivers what
> >> they can expect to hear from the bridge --- for example that the bridge can
> >> issue set / get controls or fmt ops to the subdev. The subdev may or may not
> >> need to be powered for those: only the subdev driver knows.
> > 
> > Hm, why should the bridge driver tell the subdev driver (I presume, that's 
> > a typo in your above sentence) what to _expect_? Isn't just calling those 
> > operations enough?
> > 
> >> This is analogous to opening the subdev node from user space. Anything else
> >> except streaming is allowed. And streaming, which for sure requires powering
> >> on the subdev, is already nicely handled by the s_stream op.
> >>
> >> What do you think?
> >>
> >> In practice the name of s_power should change, as well as possible
> >> implementatio on subdev drivers.
> > 
> > But why do we need it at all?
> 
> AFAICS in some TV card drivers it is used to put the analog tuner into low
> power state.
> So core.s_power op provides the mans to suspend/resume a sub-device.
> 
> If the bridge driver only implements a user space interface for the subdev,
> it may want to bring a subdev up in some specific moment, before video.s_stream,
> e.g. in some ioctl or at device open(), etc.
> 
> Let's imagine bringing the sensor up takes appr. 700 ms, often we don't want 
> the sensor driver to be doing this before every s_stream().

Sorry, I still don't understand, how the bridge driver knows better, than 
the subdev driver, whether the user will resume streaming in 500ms or in 
20s? Maybe there's some such information available with tuners, which I'm 
just unaware about?

> I agree the subdev drivers have best knowledge as to when to enable/disable
> power, from their device point of view. 
> 
> But how could we resolve the high latencies issue without something like 
> the s_power() callback ?
> 
> > 
> >>> All the above makes me think, that .s_power() methods are actually useless 
> >>> in the "operation context." The bridge has basically no way to know, when 
> >>> and which parts of the subdevice to power on or off. Subdevice 
> >>> configuration is anyway always performed, using the driver, and for 
> >>> streaming all participating subdevices just have to be informed about 
> >>> streaming begin and end.
> >>>
> >>> The only pure PM activity, that subdevice drivers have to be informed 
> >>> about are suspends and resumes. Normal bus PM callbacks are not always 
> >>> usable in our case. E.g., you cannot use i2c PM, because i2c can well be 
> >>> resumed before the bridge and then camera sensors typically still cannot 
> >>> be accessed over i2c.
> >>
> >> Do you have a bridge that provides a clock to subdevs? The clock should be
> >> modelled in the clock framework --- yes, I guess there's still a way to go
> >> before that,s universally possible.
> > 
> > sure, almost all cameras in my configurations get their master clock from 
> > the bridge - usually a dedicated camera master clock output.
> > 
> >>> Therefore I propose to either deprecate (and later remove) .s_power() and 
> >>> add .suspend() and .resume() instead or repurpose .s_power() to be _only_ 
> >>> used for system-wide suspending and resuming. Even for runtime PM the 
> >>> subdevice driver has all the chances to decide itself when and how to save 
> >>> power, so, again, there is no need to be called from outside.
> 
> Although more explicit, two suspend/resume callbacks in place of single s_power
> doesn't seem to add much value to me.

No, the important point was - to _only_ use .s_power() for system-wide PM, 
whether we keep the names or not.

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC] subdevice PM: .s_power() deprecation?
  2011-10-17 13:49       ` Guennadi Liakhovetski
@ 2011-10-17 15:15         ` Sylwester Nawrocki
  2011-10-17 15:23           ` Guennadi Liakhovetski
  0 siblings, 1 reply; 21+ messages in thread
From: Sylwester Nawrocki @ 2011-10-17 15:15 UTC (permalink / raw)
  To: Guennadi Liakhovetski
  Cc: Sakari Ailus, Linux Media Mailing List, Laurent Pinchart

On 10/17/2011 03:49 PM, Guennadi Liakhovetski wrote:
> On Mon, 17 Oct 2011, Sylwester Nawrocki wrote:
>> On 10/17/2011 10:06 AM, Guennadi Liakhovetski wrote:
>>> On Sun, 9 Oct 2011, Sakari Ailus wrote:
>>>> On Mon, Oct 03, 2011 at 12:57:10PM +0200, Guennadi Liakhovetski wrote:
...
>>>> The bridge driver can't (nor should) know about the power management
>>>> requirements of random subdevs. The name of the s_power op is rather
>>>> poitless in its current state.
>>>>
>>>> The power state of the subdev probably even never matters to the bridge ---
>>>
>>> Exactly, that's the conclusion I come to in this RFC too.
>>>
>>>> or do we really have an example of that?
>>>>
>>>> In my opinion the bridge driver should instead tell the bridge drivers what
>>>> they can expect to hear from the bridge --- for example that the bridge can
>>>> issue set / get controls or fmt ops to the subdev. The subdev may or may not
>>>> need to be powered for those: only the subdev driver knows.
>>>
>>> Hm, why should the bridge driver tell the subdev driver (I presume, that's 
>>> a typo in your above sentence) what to _expect_? Isn't just calling those 
>>> operations enough?
>>>
>>>> This is analogous to opening the subdev node from user space. Anything else
>>>> except streaming is allowed. And streaming, which for sure requires powering
>>>> on the subdev, is already nicely handled by the s_stream op.
>>>>
>>>> What do you think?
>>>>
>>>> In practice the name of s_power should change, as well as possible
>>>> implementatio on subdev drivers.
>>>
>>> But why do we need it at all?
>>
>> AFAICS in some TV card drivers it is used to put the analog tuner into low
>> power state.
>> So core.s_power op provides the mans to suspend/resume a sub-device.
>>
>> If the bridge driver only implements a user space interface for the subdev,
>> it may want to bring a subdev up in some specific moment, before video.s_stream,
>> e.g. in some ioctl or at device open(), etc.
>>
>> Let's imagine bringing the sensor up takes appr. 700 ms, often we don't want 
>> the sensor driver to be doing this before every s_stream().
> 
> Sorry, I still don't understand, how the bridge driver knows better, than 
> the subdev driver, whether the user will resume streaming in 500ms or in 
> 20s? Maybe there's some such information available with tuners, which I'm 
> just unaware about?

What I meant was that if the bridge driver assumes in advance that enabling
sensor's power and getting it fully operational takes long time, it can enable
sensor's power earlier than it's really necessary, to avoid excessive latencies
during further actions.

The bridge driver could also choose to keep the sensor powered on, whenever it
sees appropriate, to avoid re-enabling the sensor to often. 

And I'm not convinced the subdev driver has all prerequisites for implementing
the power control policy.


Regards,
-- 
Sylwester Nawrocki
Samsung Poland R&D Center

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC] subdevice PM: .s_power() deprecation?
  2011-10-17 15:15         ` Sylwester Nawrocki
@ 2011-10-17 15:23           ` Guennadi Liakhovetski
  2011-10-17 21:26             ` Sylwester Nawrocki
  0 siblings, 1 reply; 21+ messages in thread
From: Guennadi Liakhovetski @ 2011-10-17 15:23 UTC (permalink / raw)
  To: Sylwester Nawrocki
  Cc: Sakari Ailus, Linux Media Mailing List, Laurent Pinchart

On Mon, 17 Oct 2011, Sylwester Nawrocki wrote:

> On 10/17/2011 03:49 PM, Guennadi Liakhovetski wrote:
> > On Mon, 17 Oct 2011, Sylwester Nawrocki wrote:
> >> On 10/17/2011 10:06 AM, Guennadi Liakhovetski wrote:
> >>> On Sun, 9 Oct 2011, Sakari Ailus wrote:
> >>>> On Mon, Oct 03, 2011 at 12:57:10PM +0200, Guennadi Liakhovetski wrote:
> ...
> >>>> The bridge driver can't (nor should) know about the power management
> >>>> requirements of random subdevs. The name of the s_power op is rather
> >>>> poitless in its current state.
> >>>>
> >>>> The power state of the subdev probably even never matters to the bridge ---
> >>>
> >>> Exactly, that's the conclusion I come to in this RFC too.
> >>>
> >>>> or do we really have an example of that?
> >>>>
> >>>> In my opinion the bridge driver should instead tell the bridge drivers what
> >>>> they can expect to hear from the bridge --- for example that the bridge can
> >>>> issue set / get controls or fmt ops to the subdev. The subdev may or may not
> >>>> need to be powered for those: only the subdev driver knows.
> >>>
> >>> Hm, why should the bridge driver tell the subdev driver (I presume, that's 
> >>> a typo in your above sentence) what to _expect_? Isn't just calling those 
> >>> operations enough?
> >>>
> >>>> This is analogous to opening the subdev node from user space. Anything else
> >>>> except streaming is allowed. And streaming, which for sure requires powering
> >>>> on the subdev, is already nicely handled by the s_stream op.
> >>>>
> >>>> What do you think?
> >>>>
> >>>> In practice the name of s_power should change, as well as possible
> >>>> implementatio on subdev drivers.
> >>>
> >>> But why do we need it at all?
> >>
> >> AFAICS in some TV card drivers it is used to put the analog tuner into low
> >> power state.
> >> So core.s_power op provides the mans to suspend/resume a sub-device.
> >>
> >> If the bridge driver only implements a user space interface for the subdev,
> >> it may want to bring a subdev up in some specific moment, before video.s_stream,
> >> e.g. in some ioctl or at device open(), etc.
> >>
> >> Let's imagine bringing the sensor up takes appr. 700 ms, often we don't want 
> >> the sensor driver to be doing this before every s_stream().
> > 
> > Sorry, I still don't understand, how the bridge driver knows better, than 
> > the subdev driver, whether the user will resume streaming in 500ms or in 
> > 20s? Maybe there's some such information available with tuners, which I'm 
> > just unaware about?
> 
> What I meant was that if the bridge driver assumes in advance that enabling
> sensor's power and getting it fully operational takes long time, it can enable
> sensor's power earlier than it's really necessary, to avoid excessive latencies
> during further actions.

Where would a bridge driver get this information from? And how would it 
know in advance, when power would be "really needed" to enable it 
"earlier?"...

> The bridge driver could also choose to keep the sensor powered on, whenever it
> sees appropriate, to avoid re-enabling the sensor to often. 

On what basis would the bridge driver make these decisions? How would it 
know in advance, when it'll have to re-enable the subdev next time?

> And I'm not convinced the subdev driver has all prerequisites for implementing
> the power control policy.

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC] subdevice PM: .s_power() deprecation?
  2011-10-17 15:23           ` Guennadi Liakhovetski
@ 2011-10-17 21:26             ` Sylwester Nawrocki
  2011-10-17 23:07               ` Laurent Pinchart
  0 siblings, 1 reply; 21+ messages in thread
From: Sylwester Nawrocki @ 2011-10-17 21:26 UTC (permalink / raw)
  To: Guennadi Liakhovetski
  Cc: Sylwester Nawrocki, Sakari Ailus, Linux Media Mailing List,
	Laurent Pinchart

On 10/17/2011 05:23 PM, Guennadi Liakhovetski wrote:
> On Mon, 17 Oct 2011, Sylwester Nawrocki wrote:
> 
>> On 10/17/2011 03:49 PM, Guennadi Liakhovetski wrote:
>>> On Mon, 17 Oct 2011, Sylwester Nawrocki wrote:
>>>> On 10/17/2011 10:06 AM, Guennadi Liakhovetski wrote:
>>>>> On Sun, 9 Oct 2011, Sakari Ailus wrote:
>>>>>> On Mon, Oct 03, 2011 at 12:57:10PM +0200, Guennadi Liakhovetski wrote:
>> ...
>>>>>> The bridge driver can't (nor should) know about the power management
>>>>>> requirements of random subdevs. The name of the s_power op is rather
>>>>>> poitless in its current state.
>>>>>>
>>>>>> The power state of the subdev probably even never matters to the bridge ---
>>>>>
>>>>> Exactly, that's the conclusion I come to in this RFC too.
>>>>>
>>>>>> or do we really have an example of that?
>>>>>>
>>>>>> In my opinion the bridge driver should instead tell the bridge drivers what
>>>>>> they can expect to hear from the bridge --- for example that the bridge can
>>>>>> issue set / get controls or fmt ops to the subdev. The subdev may or may not
>>>>>> need to be powered for those: only the subdev driver knows.
>>>>>
>>>>> Hm, why should the bridge driver tell the subdev driver (I presume, that's
>>>>> a typo in your above sentence) what to _expect_? Isn't just calling those
>>>>> operations enough?
>>>>>
>>>>>> This is analogous to opening the subdev node from user space. Anything else
>>>>>> except streaming is allowed. And streaming, which for sure requires powering
>>>>>> on the subdev, is already nicely handled by the s_stream op.
>>>>>>
>>>>>> What do you think?
>>>>>>
>>>>>> In practice the name of s_power should change, as well as possible
>>>>>> implementatio on subdev drivers.
>>>>>
>>>>> But why do we need it at all?
>>>>
>>>> AFAICS in some TV card drivers it is used to put the analog tuner into low
>>>> power state.
>>>> So core.s_power op provides the mans to suspend/resume a sub-device.
>>>>
>>>> If the bridge driver only implements a user space interface for the subdev,
>>>> it may want to bring a subdev up in some specific moment, before video.s_stream,
>>>> e.g. in some ioctl or at device open(), etc.
>>>>
>>>> Let's imagine bringing the sensor up takes appr. 700 ms, often we don't want
>>>> the sensor driver to be doing this before every s_stream().
>>>
>>> Sorry, I still don't understand, how the bridge driver knows better, than
>>> the subdev driver, whether the user will resume streaming in 500ms or in
>>> 20s? Maybe there's some such information available with tuners, which I'm
>>> just unaware about?
>>
>> What I meant was that if the bridge driver assumes in advance that enabling
>> sensor's power and getting it fully operational takes long time, it can enable
>> sensor's power earlier than it's really necessary, to avoid excessive latencies
>> during further actions.
> 
> Where would a bridge driver get this information from? And how would it
> know in advance, when power would be "really needed" to enable it
> "earlier?"...

I don't think this information could now be retrieved from a subdev in standard way.

If a bridge driver wants low latencies it can simply be coded to issue s_power(1)
before any other op call, and s_power(0) when it's done with a subdev.

> 
>> The bridge driver could also choose to keep the sensor powered on, whenever it
>> sees appropriate, to avoid re-enabling the sensor to often.
> 
> On what basis would the bridge driver make these decisions? How would it
> know in advance, when it'll have to re-enable the subdev next time?

Re-enabling by allowing a subdev driver to entirely control the power state.
The sensor might implement "lowest power consumption" policy, while the user
might want "highest performance".
I'm referring only to camera sensor subdevs, as I don't have much experience
with other ones.   

Also there are some devices where you want to model power control explicitly,
and it is critical to overall system operation. The s5p-tv driver is one example
of these. The host driver knows exactly how the power state of its subdevs
should be handled. 
The situation with single subdev and a bridge driver may look like it could get
away without s_power(), but where multiple subdevs are involved I expect the
driver writers will want to be able to control the power state.

It seems s_power() is now used not only in suspend/resume paths so removing it for
new core.suspend/resume callbacks IMHO would leave some functionality not covered.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC] subdevice PM: .s_power() deprecation?
  2011-10-17 21:26             ` Sylwester Nawrocki
@ 2011-10-17 23:07               ` Laurent Pinchart
  2011-10-18 21:10                 ` Sylwester Nawrocki
  0 siblings, 1 reply; 21+ messages in thread
From: Laurent Pinchart @ 2011-10-17 23:07 UTC (permalink / raw)
  To: Sylwester Nawrocki
  Cc: Guennadi Liakhovetski, Sylwester Nawrocki, Sakari Ailus,
	Linux Media Mailing List

Hi Sylwester,

On Monday 17 October 2011 23:26:28 Sylwester Nawrocki wrote:
> On 10/17/2011 05:23 PM, Guennadi Liakhovetski wrote:
> > On Mon, 17 Oct 2011, Sylwester Nawrocki wrote:
> >> On 10/17/2011 03:49 PM, Guennadi Liakhovetski wrote:
> >>> On Mon, 17 Oct 2011, Sylwester Nawrocki wrote:
> >>>> On 10/17/2011 10:06 AM, Guennadi Liakhovetski wrote:
> >>>>> On Sun, 9 Oct 2011, Sakari Ailus wrote:
> >>>>>> On Mon, Oct 03, 2011 at 12:57:10PM +0200, Guennadi Liakhovetski 
wrote:
> >> ...
> >> 
> >>>>>> The bridge driver can't (nor should) know about the power management
> >>>>>> requirements of random subdevs. The name of the s_power op is rather
> >>>>>> poitless in its current state.
> >>>>>> 
> >>>>>> The power state of the subdev probably even never matters to the
> >>>>>> bridge ---
> >>>>> 
> >>>>> Exactly, that's the conclusion I come to in this RFC too.
> >>>>> 
> >>>>>> or do we really have an example of that?
> >>>>>> 
> >>>>>> In my opinion the bridge driver should instead tell the bridge
> >>>>>> drivers what they can expect to hear from the bridge --- for
> >>>>>> example that the bridge can issue set / get controls or fmt ops to
> >>>>>> the subdev. The subdev may or may not need to be powered for those:
> >>>>>> only the subdev driver knows.
> >>>>> 
> >>>>> Hm, why should the bridge driver tell the subdev driver (I presume,
> >>>>> that's a typo in your above sentence) what to _expect_? Isn't just
> >>>>> calling those operations enough?
> >>>>> 
> >>>>>> This is analogous to opening the subdev node from user space.
> >>>>>> Anything else except streaming is allowed. And streaming, which for
> >>>>>> sure requires powering on the subdev, is already nicely handled by
> >>>>>> the s_stream op.
> >>>>>> 
> >>>>>> What do you think?
> >>>>>> 
> >>>>>> In practice the name of s_power should change, as well as possible
> >>>>>> implementatio on subdev drivers.
> >>>>> 
> >>>>> But why do we need it at all?
> >>>> 
> >>>> AFAICS in some TV card drivers it is used to put the analog tuner into
> >>>> low power state.
> >>>> So core.s_power op provides the mans to suspend/resume a sub-device.
> >>>> 
> >>>> If the bridge driver only implements a user space interface for the
> >>>> subdev, it may want to bring a subdev up in some specific moment,
> >>>> before video.s_stream, e.g. in some ioctl or at device open(), etc.
> >>>> 
> >>>> Let's imagine bringing the sensor up takes appr. 700 ms, often we
> >>>> don't want the sensor driver to be doing this before every
> >>>> s_stream().
> >>> 
> >>> Sorry, I still don't understand, how the bridge driver knows better,
> >>> than the subdev driver, whether the user will resume streaming in
> >>> 500ms or in 20s? Maybe there's some such information available with
> >>> tuners, which I'm just unaware about?
> >> 
> >> What I meant was that if the bridge driver assumes in advance that
> >> enabling sensor's power and getting it fully operational takes long
> >> time, it can enable sensor's power earlier than it's really necessary,
> >> to avoid excessive latencies during further actions.
> > 
> > Where would a bridge driver get this information from? And how would it
> > know in advance, when power would be "really needed" to enable it
> > "earlier?"...
> 
> I don't think this information could now be retrieved from a subdev in
> standard way.
>
> If a bridge driver wants low latencies it can simply be coded to issue
> s_power(1) before any other op call, and s_power(0) when it's done with a
> subdev.
> 
> >> The bridge driver could also choose to keep the sensor powered on,
> >> whenever it sees appropriate, to avoid re-enabling the sensor to often.
> > 
> > On what basis would the bridge driver make these decisions? How would it
> > know in advance, when it'll have to re-enable the subdev next time?
> 
> Re-enabling by allowing a subdev driver to entirely control the power
> state. The sensor might implement "lowest power consumption" policy, while
> the user might want "highest performance".

Exactly, that's a policy decision. Would PM QoS help here ?

> I'm referring only to camera sensor subdevs, as I don't have much experience
> with other ones.
> 
> Also there are some devices where you want to model power control
> explicitly, and it is critical to overall system operation. The s5p-tv
> driver is one example of these. The host driver knows exactly how the
> power state of its subdevs should be handled.

The host probably knows about how to handle the power state of its internal 
subdevs, but what about external ones ?

> The situation with single subdev and a bridge driver may look like it could
> get away without s_power(), but where multiple subdevs are involved I
> expect the driver writers will want to be able to control the power state.
> 
> It seems s_power() is now used not only in suspend/resume paths so removing
> it for new core.suspend/resume callbacks IMHO would leave some
> functionality not covered.

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC] subdevice PM: .s_power() deprecation?
  2011-10-17 23:07               ` Laurent Pinchart
@ 2011-10-18 21:10                 ` Sylwester Nawrocki
  2011-10-18 21:38                   ` Guennadi Liakhovetski
  0 siblings, 1 reply; 21+ messages in thread
From: Sylwester Nawrocki @ 2011-10-18 21:10 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Guennadi Liakhovetski, Sakari Ailus, Linux Media Mailing List

Hi Laurent,

On 10/18/2011 01:07 AM, Laurent Pinchart wrote:
> On Monday 17 October 2011 23:26:28 Sylwester Nawrocki wrote:
>> On 10/17/2011 05:23 PM, Guennadi Liakhovetski wrote:
>>> On Mon, 17 Oct 2011, Sylwester Nawrocki wrote:
>>>> On 10/17/2011 03:49 PM, Guennadi Liakhovetski wrote:
>>>>> On Mon, 17 Oct 2011, Sylwester Nawrocki wrote:
>>>>>> On 10/17/2011 10:06 AM, Guennadi Liakhovetski wrote:
>>>>>>> On Sun, 9 Oct 2011, Sakari Ailus wrote:
>>>>>>>> On Mon, Oct 03, 2011 at 12:57:10PM +0200, Guennadi Liakhovetski
> wrote:
>>>> ...
>>>>
>>>>>>>> The bridge driver can't (nor should) know about the power management
>>>>>>>> requirements of random subdevs. The name of the s_power op is rather
>>>>>>>> poitless in its current state.
>>>>>>>>
>>>>>>>> The power state of the subdev probably even never matters to the
>>>>>>>> bridge ---
>>>>>>>
>>>>>>> Exactly, that's the conclusion I come to in this RFC too.
>>>>>>>
>>>>>>>> or do we really have an example of that?
>>>>>>>>
>>>>>>>> In my opinion the bridge driver should instead tell the bridge
>>>>>>>> drivers what they can expect to hear from the bridge --- for
>>>>>>>> example that the bridge can issue set / get controls or fmt ops to
>>>>>>>> the subdev. The subdev may or may not need to be powered for those:
>>>>>>>> only the subdev driver knows.
>>>>>>>
>>>>>>> Hm, why should the bridge driver tell the subdev driver (I presume,
>>>>>>> that's a typo in your above sentence) what to _expect_? Isn't just
>>>>>>> calling those operations enough?
>>>>>>>
>>>>>>>> This is analogous to opening the subdev node from user space.
>>>>>>>> Anything else except streaming is allowed. And streaming, which for
>>>>>>>> sure requires powering on the subdev, is already nicely handled by
>>>>>>>> the s_stream op.
>>>>>>>>
>>>>>>>> What do you think?
>>>>>>>>
>>>>>>>> In practice the name of s_power should change, as well as possible
>>>>>>>> implementatio on subdev drivers.
>>>>>>>
>>>>>>> But why do we need it at all?
>>>>>>
>>>>>> AFAICS in some TV card drivers it is used to put the analog tuner into
>>>>>> low power state.
>>>>>> So core.s_power op provides the mans to suspend/resume a sub-device.
>>>>>>
>>>>>> If the bridge driver only implements a user space interface for the
>>>>>> subdev, it may want to bring a subdev up in some specific moment,
>>>>>> before video.s_stream, e.g. in some ioctl or at device open(), etc.
>>>>>>
>>>>>> Let's imagine bringing the sensor up takes appr. 700 ms, often we
>>>>>> don't want the sensor driver to be doing this before every
>>>>>> s_stream().
>>>>>
>>>>> Sorry, I still don't understand, how the bridge driver knows better,
>>>>> than the subdev driver, whether the user will resume streaming in
>>>>> 500ms or in 20s? Maybe there's some such information available with
>>>>> tuners, which I'm just unaware about?
>>>>
>>>> What I meant was that if the bridge driver assumes in advance that
>>>> enabling sensor's power and getting it fully operational takes long
>>>> time, it can enable sensor's power earlier than it's really necessary,
>>>> to avoid excessive latencies during further actions.
>>>
>>> Where would a bridge driver get this information from? And how would it
>>> know in advance, when power would be "really needed" to enable it
>>> "earlier?"...
>>
>> I don't think this information could now be retrieved from a subdev in
>> standard way.
>>
>> If a bridge driver wants low latencies it can simply be coded to issue
>> s_power(1) before any other op call, and s_power(0) when it's done with a
>> subdev.
>>
>>>> The bridge driver could also choose to keep the sensor powered on,
>>>> whenever it sees appropriate, to avoid re-enabling the sensor to often.
>>>
>>> On what basis would the bridge driver make these decisions? How would it
>>> know in advance, when it'll have to re-enable the subdev next time?
>>
>> Re-enabling by allowing a subdev driver to entirely control the power
>> state. The sensor might implement "lowest power consumption" policy, while
>> the user might want "highest performance".
> 
> Exactly, that's a policy decision. Would PM QoS help here ?

Thanks for reminding about PM QoS. I didn't pay much attention to it but it
indeed appears to be a good fit for this sort of tasks.

We would possibly just need to think of parameters which could be associated with
video, e.g. video_latency, etc. ?...

I'm curious whether the whole power handling could be contained within a subdev 
driver, most likely it could be done for subdevs exposing a devnode.

> 
>> I'm referring only to camera sensor subdevs, as I don't have much experience
>> with other ones.
>>
>> Also there are some devices where you want to model power control
>> explicitly, and it is critical to overall system operation. The s5p-tv
>> driver is one example of these. The host driver knows exactly how the
>> power state of its subdevs should be handled.
> 
> The host probably knows about how to handle the power state of its internal
> subdevs, but what about external ones ?

In this particular example there is no external subdevs associated with the host. 

But we don't seem to have separate callbacks for internal and external subdevs..
So removing s_power() puts the above described sort of drivers in trouble.

I guess we all agree the power requirements of external subdevs are generally 
unknown to the hosts.

For these it might make lot of sense to let the subdev driver handle the device
power supplies on basis of requests like, s_ctrl, s_stream, etc.  

With PM QoS it could be easier to decide in the driver when a device should be
put in a low power state. 

---
Regards,
Sylwester

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC] subdevice PM: .s_power() deprecation?
  2011-10-18 21:10                 ` Sylwester Nawrocki
@ 2011-10-18 21:38                   ` Guennadi Liakhovetski
  2011-10-19 20:56                     ` Sylwester Nawrocki
  2011-10-20  9:45                     ` Laurent Pinchart
  0 siblings, 2 replies; 21+ messages in thread
From: Guennadi Liakhovetski @ 2011-10-18 21:38 UTC (permalink / raw)
  To: Sylwester Nawrocki
  Cc: Laurent Pinchart, Sakari Ailus, Linux Media Mailing List

On Tue, 18 Oct 2011, Sylwester Nawrocki wrote:

> Hi Laurent,
> 
> On 10/18/2011 01:07 AM, Laurent Pinchart wrote:
> > On Monday 17 October 2011 23:26:28 Sylwester Nawrocki wrote:
> >> On 10/17/2011 05:23 PM, Guennadi Liakhovetski wrote:
> >>> On Mon, 17 Oct 2011, Sylwester Nawrocki wrote:

[snip]

> >>>> The bridge driver could also choose to keep the sensor powered on,
> >>>> whenever it sees appropriate, to avoid re-enabling the sensor to often.
> >>>
> >>> On what basis would the bridge driver make these decisions? How would it
> >>> know in advance, when it'll have to re-enable the subdev next time?
> >>
> >> Re-enabling by allowing a subdev driver to entirely control the power
> >> state. The sensor might implement "lowest power consumption" policy, while
> >> the user might want "highest performance".
> > 
> > Exactly, that's a policy decision. Would PM QoS help here ?
> 
> Thanks for reminding about PM QoS. I didn't pay much attention to it but it
> indeed appears to be a good fit for this sort of tasks.

But you anyway have to decide - who will implement those PM QoS callbacks? 
The bridge and then decide whether or not to call subdev's .s_power(), or 
the subdev driver itself? I think, the latter, in which case .s_power() 
remain unused.

> We would possibly just need to think of parameters which could be associated with
> video, e.g. video_latency, etc. ?...
> 
> I'm curious whether the whole power handling could be contained within a subdev 
> driver, most likely it could be done for subdevs exposing a devnode.
> 
> > 
> >> I'm referring only to camera sensor subdevs, as I don't have much experience
> >> with other ones.
> >>
> >> Also there are some devices where you want to model power control
> >> explicitly, and it is critical to overall system operation. The s5p-tv
> >> driver is one example of these. The host driver knows exactly how the
> >> power state of its subdevs should be handled.
> > 
> > The host probably knows about how to handle the power state of its internal
> > subdevs, but what about external ones ?
> 
> In this particular example there is no external subdevs associated with the host. 
> 
> But we don't seem to have separate callbacks for internal and external subdevs..
> So removing s_power() puts the above described sort of drivers in trouble.

I understand what you're saying, but can you give us a specific example, 
when a subdev driver (your SoC internal subdev, that is) doesn't have a 
way to react to an event itself and only the bridge driver gets called 
into at that time? Something like an interrupt or an internal timer or 
some other internal event?

> I guess we all agree the power requirements of external subdevs are generally 
> unknown to the hosts.
> 
> For these it might make lot of sense to let the subdev driver handle the device
> power supplies on basis of requests like, s_ctrl, s_stream, etc.  

Yes, right, so, most "external" (sensor, decoder,...) subdev drivers 
should never need to implement .s_power(), regardless of whether we decide 
to keep it or not. Well, ok, no, put it differently - in those drivers 
.s_power() should only really be called during system-wide suspend / 
resume.

> With PM QoS it could be easier to decide in the driver when a device should be
> put in a low power state. 

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC] subdevice PM: .s_power() deprecation?
  2011-10-18 21:38                   ` Guennadi Liakhovetski
@ 2011-10-19 20:56                     ` Sylwester Nawrocki
  2011-10-23  8:07                       ` Sakari Ailus
  2011-10-20  9:45                     ` Laurent Pinchart
  1 sibling, 1 reply; 21+ messages in thread
From: Sylwester Nawrocki @ 2011-10-19 20:56 UTC (permalink / raw)
  To: Guennadi Liakhovetski
  Cc: Laurent Pinchart, Sakari Ailus, Linux Media Mailing List,
	Tomasz Stanislawski

On 10/18/2011 11:38 PM, Guennadi Liakhovetski wrote:
> On Tue, 18 Oct 2011, Sylwester Nawrocki wrote:
>> On 10/18/2011 01:07 AM, Laurent Pinchart wrote:
>>> On Monday 17 October 2011 23:26:28 Sylwester Nawrocki wrote:
>>>> On 10/17/2011 05:23 PM, Guennadi Liakhovetski wrote:
>>>>> On Mon, 17 Oct 2011, Sylwester Nawrocki wrote:
> 
> [snip]
> 
>>>>>> The bridge driver could also choose to keep the sensor powered on,
>>>>>> whenever it sees appropriate, to avoid re-enabling the sensor to often.
>>>>>
>>>>> On what basis would the bridge driver make these decisions? How would it
>>>>> know in advance, when it'll have to re-enable the subdev next time?
>>>>
>>>> Re-enabling by allowing a subdev driver to entirely control the power
>>>> state. The sensor might implement "lowest power consumption" policy, while
>>>> the user might want "highest performance".
>>>
>>> Exactly, that's a policy decision. Would PM QoS help here ?
>>
>> Thanks for reminding about PM QoS. I didn't pay much attention to it but it
>> indeed appears to be a good fit for this sort of tasks.
> 
> But you anyway have to decide - who will implement those PM QoS callbacks?
> The bridge and then decide whether or not to call subdev's .s_power(), or
> the subdev driver itself? I think, the latter, in which case .s_power()
> remain unused.

Agreed. With some hints on how to handle power supply directly in the subdev
driver we should be able to do without s_power() at the "edge" subdevs.

> 
>> We would possibly just need to think of parameters which could be associated with
>> video, e.g. video_latency, etc. ?...
>>
>> I'm curious whether the whole power handling could be contained within a subdev
>> driver, most likely it could be done for subdevs exposing a devnode.
>>
>>>
>>>> I'm referring only to camera sensor subdevs, as I don't have much experience
>>>> with other ones.
>>>>
>>>> Also there are some devices where you want to model power control
>>>> explicitly, and it is critical to overall system operation. The s5p-tv
>>>> driver is one example of these. The host driver knows exactly how the
>>>> power state of its subdevs should be handled.
>>>
>>> The host probably knows about how to handle the power state of its internal
>>> subdevs, but what about external ones ?
>>
>> In this particular example there is no external subdevs associated with the host.
>>
>> But we don't seem to have separate callbacks for internal and external subdevs..
>> So removing s_power() puts the above described sort of drivers in trouble.
> 
> I understand what you're saying, but can you give us a specific example,
> when a subdev driver (your SoC internal subdev, that is) doesn't have a
> way to react to an event itself and only the bridge driver gets called
> into at that time? Something like an interrupt or an internal timer or
> some other internal event?

1. The S5P SoC video output subsystem (http://lwn.net/Articles/449661) comprises
 of multiple logical blocks, like Video Processor, Mixer, HDMI, HDMI PHY, SD TV Out.
 For instance the master video clock is during normal operation derived from
 (synchronized to, with PLL,) the HDMI-PHY output clock. The host driver can
 switch to this clock only when the HDMI-PHY (subdev) power and clocks are enabled.
 And it should be done before .s_stream(), to do some H/W configuration earlier
 in the pipeline, before streaming is enabled. Perhaps Tomasz could give some
 further explanation of what the s_power() op does and why in the driver. 
 
2. In some of our camera pipeline setups - "Sensor - MIPI-CSI receiver - host/DMA",
 the sensor won't boot properly if all MIPI-CSI regulators aren't enabled. So the  
 MIPI-CSI receiver must always be powered on before the sensor. With the subdevs
 doing their own magic wrt to power control the situation is getting slightly
 out of control. 
 
> 
>> I guess we all agree the power requirements of external subdevs are generally
>> unknown to the hosts.
>>
>> For these it might make lot of sense to let the subdev driver handle the device
>> power supplies on basis of requests like, s_ctrl, s_stream, etc.
> 
> Yes, right, so, most "external" (sensor, decoder,...) subdev drivers
> should never need to implement .s_power(), regardless of whether we decide
> to keep it or not. Well, ok, no, put it differently - in those drivers
> .s_power() should only really be called during system-wide suspend /
> resume.

Yes, I agree with that. But before we attempt to remove .s_power() or deprecate 
it on "external" subdevs, I'd like to get solved the issue with sensor master clock 
provided by the bridge. As these two are closely related - the sensor controller 
won't boot if the clock is disabled. And there are always advantages of not keeping
the clock always enabled. 

> 
>> With PM QoS it could be easier to decide in the driver when a device should be
>> put in a low power state.

-- 
Regards,
Sylwester

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC] subdevice PM: .s_power() deprecation?
  2011-10-18 21:38                   ` Guennadi Liakhovetski
  2011-10-19 20:56                     ` Sylwester Nawrocki
@ 2011-10-20  9:45                     ` Laurent Pinchart
  1 sibling, 0 replies; 21+ messages in thread
From: Laurent Pinchart @ 2011-10-20  9:45 UTC (permalink / raw)
  To: Guennadi Liakhovetski
  Cc: Sylwester Nawrocki, Sakari Ailus, Linux Media Mailing List

Hi Guennadi,

On Tuesday 18 October 2011 23:38:11 Guennadi Liakhovetski wrote:
> On Tue, 18 Oct 2011, Sylwester Nawrocki wrote:
> > On 10/18/2011 01:07 AM, Laurent Pinchart wrote:
> > > On Monday 17 October 2011 23:26:28 Sylwester Nawrocki wrote:
> > >> On 10/17/2011 05:23 PM, Guennadi Liakhovetski wrote:
> > >>> On Mon, 17 Oct 2011, Sylwester Nawrocki wrote:
> [snip]
> 
> > >>>> The bridge driver could also choose to keep the sensor powered on,
> > >>>> whenever it sees appropriate, to avoid re-enabling the sensor to
> > >>>> often.
> > >>> 
> > >>> On what basis would the bridge driver make these decisions? How would
> > >>> it know in advance, when it'll have to re-enable the subdev next
> > >>> time?
> > >> 
> > >> Re-enabling by allowing a subdev driver to entirely control the power
> > >> state. The sensor might implement "lowest power consumption" policy,
> > >> while the user might want "highest performance".
> > > 
> > > Exactly, that's a policy decision. Would PM QoS help here ?
> > 
> > Thanks for reminding about PM QoS. I didn't pay much attention to it but
> > it indeed appears to be a good fit for this sort of tasks.
> 
> But you anyway have to decide - who will implement those PM QoS callbacks?
> The bridge and then decide whether or not to call subdev's .s_power(), or
> the subdev driver itself? I think, the latter, in which case .s_power()
> remain unused.

With a proper PM QoS framework in place, the .s_power() operation might 
disappear. However we can't get rid of it today, as PM QoS isn't there yet.

As I mentioned before, low-latency requires decoupling sensor power on and 
video stream startup. The OMAP3 ISP driver currently implements such a low-
latency policy by calling .s_power(1) on the sensor when a video node 
belonging to the same pipeline as the sensor is opened. This is not an ideal 
solution, but removing .s_power() would prevent that without providing any 
other mean to achieve the same low-latency policy.

> > We would possibly just need to think of parameters which could be
> > associated with video, e.g. video_latency, etc. ?...
> > 
> > I'm curious whether the whole power handling could be contained within a
> > subdev driver, most likely it could be done for subdevs exposing a
> > devnode.
> > 
> > >> I'm referring only to camera sensor subdevs, as I don't have much
> > >> experience with other ones.
> > >> 
> > >> Also there are some devices where you want to model power control
> > >> explicitly, and it is critical to overall system operation. The s5p-tv
> > >> driver is one example of these. The host driver knows exactly how the
> > >> power state of its subdevs should be handled.
> > > 
> > > The host probably knows about how to handle the power state of its
> > > internal subdevs, but what about external ones ?
> > 
> > In this particular example there is no external subdevs associated with
> > the host.
> > 
> > But we don't seem to have separate callbacks for internal and external
> > subdevs.. So removing s_power() puts the above described sort of drivers
> > in trouble.
> 
> I understand what you're saying, but can you give us a specific example,
> when a subdev driver (your SoC internal subdev, that is) doesn't have a
> way to react to an event itself and only the bridge driver gets called
> into at that time? Something like an interrupt or an internal timer or
> some other internal event?

See above :-)

> > I guess we all agree the power requirements of external subdevs are
> > generally unknown to the hosts.
> > 
> > For these it might make lot of sense to let the subdev driver handle the
> > device power supplies on basis of requests like, s_ctrl, s_stream, etc.
> 
> Yes, right, so, most "external" (sensor, decoder,...) subdev drivers
> should never need to implement .s_power(), regardless of whether we decide
> to keep it or not. Well, ok, no, put it differently - in those drivers
> .s_power() should only really be called during system-wide suspend /
> resume.

With a PM QoS framework, that's right.

> > With PM QoS it could be easier to decide in the driver when a device
> > should be put in a low power state.

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC] subdevice PM: .s_power() deprecation?
  2011-10-19 20:56                     ` Sylwester Nawrocki
@ 2011-10-23  8:07                       ` Sakari Ailus
  2011-10-23  8:35                         ` Sylwester Nawrocki
  0 siblings, 1 reply; 21+ messages in thread
From: Sakari Ailus @ 2011-10-23  8:07 UTC (permalink / raw)
  To: Sylwester Nawrocki
  Cc: Guennadi Liakhovetski, Laurent Pinchart, Linux Media Mailing List,
	Tomasz Stanislawski

Hi Sylwester,

Sylwester Nawrocki wrote:
...
>> I understand what you're saying, but can you give us a specific example,
>> when a subdev driver (your SoC internal subdev, that is) doesn't have a
>> way to react to an event itself and only the bridge driver gets called
>> into at that time? Something like an interrupt or an internal timer or
>> some other internal event?
> 
> 1. The S5P SoC video output subsystem (http://lwn.net/Articles/449661) comprises
>  of multiple logical blocks, like Video Processor, Mixer, HDMI, HDMI PHY, SD TV Out.
>  For instance the master video clock is during normal operation derived from
>  (synchronized to, with PLL,) the HDMI-PHY output clock. The host driver can
>  switch to this clock only when the HDMI-PHY (subdev) power and clocks are enabled.
>  And it should be done before .s_stream(), to do some H/W configuration earlier
>  in the pipeline, before streaming is enabled. Perhaps Tomasz could give some
>  further explanation of what the s_power() op does and why in the driver. 
>  
> 2. In some of our camera pipeline setups - "Sensor - MIPI-CSI receiver - host/DMA",
>  the sensor won't boot properly if all MIPI-CSI regulators aren't enabled. So the  
>  MIPI-CSI receiver must always be powered on before the sensor. With the subdevs
>  doing their own magic wrt to power control the situation is getting slightly
>  out of control. 

How about this: CSI-2 receiver implements a few new regulators which the
sensor driver then requests to be enabled. Would that work for you?

>>> I guess we all agree the power requirements of external subdevs are generally
>>> unknown to the hosts.
>>>
>>> For these it might make lot of sense to let the subdev driver handle the device
>>> power supplies on basis of requests like, s_ctrl, s_stream, etc.
>>
>> Yes, right, so, most "external" (sensor, decoder,...) subdev drivers
>> should never need to implement .s_power(), regardless of whether we decide
>> to keep it or not. Well, ok, no, put it differently - in those drivers
>> .s_power() should only really be called during system-wide suspend /
>> resume.
> 
> Yes, I agree with that. But before we attempt to remove .s_power() or deprecate 
> it on "external" subdevs, I'd like to get solved the issue with sensor master clock 
> provided by the bridge. As these two are closely related - the sensor controller 
> won't boot if the clock is disabled. And there are always advantages of not keeping
> the clock always enabled. 

I guess we'll need to wait awhile before the clock framework would
support this. I don't know what's the status of this; probably worth
checking.

Regards,

-- 
Sakari Ailus
sakari.ailus@iki.fi

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC] subdevice PM: .s_power() deprecation?
  2011-10-23  8:07                       ` Sakari Ailus
@ 2011-10-23  8:35                         ` Sylwester Nawrocki
  2011-10-23  8:44                           ` Sakari Ailus
  0 siblings, 1 reply; 21+ messages in thread
From: Sylwester Nawrocki @ 2011-10-23  8:35 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: Guennadi Liakhovetski, Laurent Pinchart, Linux Media Mailing List,
	Tomasz Stanislawski

Hi Sakari,

On 10/23/2011 10:07 AM, Sakari Ailus wrote:
> Sylwester Nawrocki wrote:
> ...
>>> I understand what you're saying, but can you give us a specific example,
>>> when a subdev driver (your SoC internal subdev, that is) doesn't have a
>>> way to react to an event itself and only the bridge driver gets called
>>> into at that time? Something like an interrupt or an internal timer or
>>> some other internal event?
>>
>> 1. The S5P SoC video output subsystem (http://lwn.net/Articles/449661) comprises
>>   of multiple logical blocks, like Video Processor, Mixer, HDMI, HDMI PHY, SD TV Out.
>>   For instance the master video clock is during normal operation derived from
>>   (synchronized to, with PLL,) the HDMI-PHY output clock. The host driver can
>>   switch to this clock only when the HDMI-PHY (subdev) power and clocks are enabled.
>>   And it should be done before .s_stream(), to do some H/W configuration earlier
>>   in the pipeline, before streaming is enabled. Perhaps Tomasz could give some
>>   further explanation of what the s_power() op does and why in the driver.
>>
>> 2. In some of our camera pipeline setups - "Sensor - MIPI-CSI receiver - host/DMA",
>>   the sensor won't boot properly if all MIPI-CSI regulators aren't enabled. So the
>>   MIPI-CSI receiver must always be powered on before the sensor. With the subdevs
>>   doing their own magic wrt to power control the situation is getting slightly
>>   out of control.
> 
> How about this: CSI-2 receiver implements a few new regulators which the
> sensor driver then requests to be enabled. Would that work for you?

No, I don't like that... :)

We would have to standardize the regulator supply names, etc. Such approach
would be more difficult to align with runtime/system suspend/resume.
Also the sensor drivers should be independent on other drivers. The MIPI-CSI
receiver is more specific to the host, rather than a sensor.

Not all sensors need MIPI-CSI, some just use parallel video bus.

> 
>>>> I guess we all agree the power requirements of external subdevs are generally
>>>> unknown to the hosts.
>>>>
>>>> For these it might make lot of sense to let the subdev driver handle the device
>>>> power supplies on basis of requests like, s_ctrl, s_stream, etc.
>>>
>>> Yes, right, so, most "external" (sensor, decoder,...) subdev drivers
>>> should never need to implement .s_power(), regardless of whether we decide
>>> to keep it or not. Well, ok, no, put it differently - in those drivers
>>> .s_power() should only really be called during system-wide suspend /
>>> resume.
>>
>> Yes, I agree with that. But before we attempt to remove .s_power() or deprecate
>> it on "external" subdevs, I'd like to get solved the issue with sensor master clock
>> provided by the bridge. As these two are closely related - the sensor controller
>> won't boot if the clock is disabled. And there are always advantages of not keeping
>> the clock always enabled.
> 
> I guess we'll need to wait awhile before the clock framework would
> support this. I don't know what's the status of this; probably worth
> checking.

Last time I checked, a reference platform migration to common clk struct was
being prepared for OMAP.
So hopefully we are close to the agreement. I think there is a speech about
this during ELCE.

--
Regards,
Sylwester

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC] subdevice PM: .s_power() deprecation?
  2011-10-23  8:35                         ` Sylwester Nawrocki
@ 2011-10-23  8:44                           ` Sakari Ailus
  2011-10-23  9:01                             ` Sylwester Nawrocki
  0 siblings, 1 reply; 21+ messages in thread
From: Sakari Ailus @ 2011-10-23  8:44 UTC (permalink / raw)
  To: Sylwester Nawrocki
  Cc: Guennadi Liakhovetski, Laurent Pinchart, Linux Media Mailing List,
	Tomasz Stanislawski

Sylwester Nawrocki wrote:
> Hi Sakari,
> 
> On 10/23/2011 10:07 AM, Sakari Ailus wrote:
>> Sylwester Nawrocki wrote:
>> ...
>>>> I understand what you're saying, but can you give us a specific example,
>>>> when a subdev driver (your SoC internal subdev, that is) doesn't have a
>>>> way to react to an event itself and only the bridge driver gets called
>>>> into at that time? Something like an interrupt or an internal timer or
>>>> some other internal event?
>>>
>>> 1. The S5P SoC video output subsystem (http://lwn.net/Articles/449661) comprises
>>>   of multiple logical blocks, like Video Processor, Mixer, HDMI, HDMI PHY, SD TV Out.
>>>   For instance the master video clock is during normal operation derived from
>>>   (synchronized to, with PLL,) the HDMI-PHY output clock. The host driver can
>>>   switch to this clock only when the HDMI-PHY (subdev) power and clocks are enabled.
>>>   And it should be done before .s_stream(), to do some H/W configuration earlier
>>>   in the pipeline, before streaming is enabled. Perhaps Tomasz could give some
>>>   further explanation of what the s_power() op does and why in the driver.
>>>
>>> 2. In some of our camera pipeline setups - "Sensor - MIPI-CSI receiver - host/DMA",
>>>   the sensor won't boot properly if all MIPI-CSI regulators aren't enabled. So the
>>>   MIPI-CSI receiver must always be powered on before the sensor. With the subdevs
>>>   doing their own magic wrt to power control the situation is getting slightly
>>>   out of control.
>>
>> How about this: CSI-2 receiver implements a few new regulators which the
>> sensor driver then requests to be enabled. Would that work for you?
> 
> No, I don't like that... :)
> 
> We would have to standardize the regulator supply names, etc. Such approach
> would be more difficult to align with runtime/system suspend/resume.
> Also the sensor drivers should be independent on other drivers. The MIPI-CSI
> receiver is more specific to the host, rather than a sensor.
> 
> Not all sensors need MIPI-CSI, some just use parallel video bus.

The sensor drivers are responsible for the regulators they want to use,
right? If they need no CSI-2 related regulators then they just ignore
them as any other regulators the sensor doesn't need.

The names of the regulators could come from the platform data, they're
board specific anyway. I can't see another way to do this without having
platform code to do this which is not quite compatible with the idea of
the device tree.

-- 
Sakari Ailus
sakari.ailus@iki.fi

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC] subdevice PM: .s_power() deprecation?
  2011-10-23  8:44                           ` Sakari Ailus
@ 2011-10-23  9:01                             ` Sylwester Nawrocki
  2011-10-23  9:26                               ` Laurent Pinchart
  0 siblings, 1 reply; 21+ messages in thread
From: Sylwester Nawrocki @ 2011-10-23  9:01 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: Guennadi Liakhovetski, Laurent Pinchart, Linux Media Mailing List,
	Tomasz Stanislawski

On 10/23/2011 10:44 AM, Sakari Ailus wrote:
> Sylwester Nawrocki wrote:
...
>>>> 2. In some of our camera pipeline setups - "Sensor - MIPI-CSI receiver - host/DMA",
>>>>    the sensor won't boot properly if all MIPI-CSI regulators aren't enabled. So the
>>>>    MIPI-CSI receiver must always be powered on before the sensor. With the subdevs
>>>>    doing their own magic wrt to power control the situation is getting slightly
>>>>    out of control.
>>>
>>> How about this: CSI-2 receiver implements a few new regulators which the
>>> sensor driver then requests to be enabled. Would that work for you?
>>
>> No, I don't like that... :)
>>
>> We would have to standardize the regulator supply names, etc. Such approach
>> would be more difficult to align with runtime/system suspend/resume.
>> Also the sensor drivers should be independent on other drivers. The MIPI-CSI
>> receiver is more specific to the host, rather than a sensor.
>>
>> Not all sensors need MIPI-CSI, some just use parallel video bus.
> 
> The sensor drivers are responsible for the regulators they want to use,
> right? If they need no CSI-2 related regulators then they just ignore

Only for the regulator supplies for their device. In this case the sensor
driver would have to touch MIPI-CSI device regulator supplies.

> them as any other regulators the sensor doesn't need.
> 
> The names of the regulators could come from the platform data, they're
> board specific anyway. I can't see another way to do this without having

No, you don't want regulator supply names in any platform data struct.
The platform code binds regulator supplies to the devices, whether it is DT
based or not.

> platform code to do this which is not quite compatible with the idea of
> the device tree.

Now I just use s_power callback in our drivers and it all works well.

--
Regards,
Sylwester

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC] subdevice PM: .s_power() deprecation?
  2011-10-23  9:01                             ` Sylwester Nawrocki
@ 2011-10-23  9:26                               ` Laurent Pinchart
  2011-10-23  9:53                                 ` Sylwester Nawrocki
  0 siblings, 1 reply; 21+ messages in thread
From: Laurent Pinchart @ 2011-10-23  9:26 UTC (permalink / raw)
  To: Sylwester Nawrocki
  Cc: Sakari Ailus, Guennadi Liakhovetski, Linux Media Mailing List,
	Tomasz Stanislawski

Hi Sylwester,

On Sunday 23 October 2011 11:01:15 Sylwester Nawrocki wrote:
> On 10/23/2011 10:44 AM, Sakari Ailus wrote:
> > Sylwester Nawrocki wrote:
> ...
> 
> >>>> 2. In some of our camera pipeline setups - "Sensor - MIPI-CSI receiver
> >>>> - host/DMA",
> >>>> 
> >>>>    the sensor won't boot properly if all MIPI-CSI regulators aren't
> >>>>    enabled. So the MIPI-CSI receiver must always be powered on before
> >>>>    the sensor. With the subdevs doing their own magic wrt to power
> >>>>    control the situation is getting slightly out of control.
> >>> 
> >>> How about this: CSI-2 receiver implements a few new regulators which
> >>> the sensor driver then requests to be enabled. Would that work for
> >>> you?
> >> 
> >> No, I don't like that... :)
> >> 
> >> We would have to standardize the regulator supply names, etc. Such
> >> approach would be more difficult to align with runtime/system
> >> suspend/resume. Also the sensor drivers should be independent on other
> >> drivers. The MIPI-CSI receiver is more specific to the host, rather
> >> than a sensor.
> >> 
> >> Not all sensors need MIPI-CSI, some just use parallel video bus.
> > 
> > The sensor drivers are responsible for the regulators they want to use,
> > right? If they need no CSI-2 related regulators then they just ignore
> 
> Only for the regulator supplies for their device. In this case the sensor
> driver would have to touch MIPI-CSI device regulator supplies.
> 
> > them as any other regulators the sensor doesn't need.
> > 
> > The names of the regulators could come from the platform data, they're
> > board specific anyway. I can't see another way to do this without having
> 
> No, you don't want regulator supply names in any platform data struct.
> The platform code binds regulator supplies to the devices, whether it is DT
> based or not.

You can still add a regulator name field to the sensor platform data, or a 
link to the regulator in the device tree, and use that in the sensor driver if 
present.

I'm not telling it's a good solution, but it's technically doable.

> > platform code to do this which is not quite compatible with the idea of
> > the device tree.
> 
> Now I just use s_power callback in our drivers and it all works well.

Having the sensor driver calling the CSI-2 receiver s_power callback directly 
sounds a bit hackish to me. If we really want to call subdev operations from 
another subdev driver we'll need to specify that, as the current mode of 
operation (at least in my understanding) is that subdev operations are only 
called by host drivers.

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC] subdevice PM: .s_power() deprecation?
  2011-10-23  9:26                               ` Laurent Pinchart
@ 2011-10-23  9:53                                 ` Sylwester Nawrocki
  0 siblings, 0 replies; 21+ messages in thread
From: Sylwester Nawrocki @ 2011-10-23  9:53 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Sylwester Nawrocki, Sakari Ailus, Guennadi Liakhovetski,
	Linux Media Mailing List, Tomasz Stanislawski

Hi Laurent,

On 10/23/2011 11:26 AM, Laurent Pinchart wrote:
> On Sunday 23 October 2011 11:01:15 Sylwester Nawrocki wrote:
>> On 10/23/2011 10:44 AM, Sakari Ailus wrote:
>>> Sylwester Nawrocki wrote:
>>>>>> 2. In some of our camera pipeline setups - "Sensor - MIPI-CSI receiver
>>>>>> - host/DMA",
>>>>>>
>>>>>>     the sensor won't boot properly if all MIPI-CSI regulators aren't
>>>>>>     enabled. So the MIPI-CSI receiver must always be powered on before
>>>>>>     the sensor. With the subdevs doing their own magic wrt to power
>>>>>>     control the situation is getting slightly out of control.
>>>>>
>>>>> How about this: CSI-2 receiver implements a few new regulators which
>>>>> the sensor driver then requests to be enabled. Would that work for
>>>>> you?
>>>>
>>>> No, I don't like that... :)
>>>>
>>>> We would have to standardize the regulator supply names, etc. Such
>>>> approach would be more difficult to align with runtime/system
>>>> suspend/resume. Also the sensor drivers should be independent on other
>>>> drivers. The MIPI-CSI receiver is more specific to the host, rather
>>>> than a sensor.
>>>>
>>>> Not all sensors need MIPI-CSI, some just use parallel video bus.
>>>
>>> The sensor drivers are responsible for the regulators they want to use,
>>> right? If they need no CSI-2 related regulators then they just ignore
>>
>> Only for the regulator supplies for their device. In this case the sensor
>> driver would have to touch MIPI-CSI device regulator supplies.
>>
>>> them as any other regulators the sensor doesn't need.
>>>
>>> The names of the regulators could come from the platform data, they're
>>> board specific anyway. I can't see another way to do this without having
>>
>> No, you don't want regulator supply names in any platform data struct.
>> The platform code binds regulator supplies to the devices, whether it is DT
>> based or not.
> 
> You can still add a regulator name field to the sensor platform data, or a
> link to the regulator in the device tree, and use that in the sensor driver if
> present.
> 
> I'm not telling it's a good solution, but it's technically doable.

Yes, that would be also possible.

> 
>>> platform code to do this which is not quite compatible with the idea of
>>> the device tree.
>>
>> Now I just use s_power callback in our drivers and it all works well.
> 
> Having the sensor driver calling the CSI-2 receiver s_power callback directly
> sounds a bit hackish to me. If we really want to call subdev operations from
> another subdev driver we'll need to specify that, as the current mode of
> operation (at least in my understanding) is that subdev operations are only
> called by host drivers.

I meant s_power() is only used by the host. Every device in the pipeline is
powered up by the host at video device open() in right order.

Maybe we could ad some kind of notifier to v4l core so the host gets notified
when any of subdevs registered to it gets it's video node opened ?

--
Regards,
Sylwester 

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2011-10-23  9:54 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-10-03 10:57 [RFC] subdevice PM: .s_power() deprecation? Guennadi Liakhovetski
2011-10-08 21:36 ` Sakari Ailus
2011-10-17  8:06   ` Guennadi Liakhovetski
2011-10-17 12:12     ` Laurent Pinchart
2011-10-17 12:37       ` Guennadi Liakhovetski
2011-10-17 12:59     ` Sylwester Nawrocki
2011-10-17 13:49       ` Guennadi Liakhovetski
2011-10-17 15:15         ` Sylwester Nawrocki
2011-10-17 15:23           ` Guennadi Liakhovetski
2011-10-17 21:26             ` Sylwester Nawrocki
2011-10-17 23:07               ` Laurent Pinchart
2011-10-18 21:10                 ` Sylwester Nawrocki
2011-10-18 21:38                   ` Guennadi Liakhovetski
2011-10-19 20:56                     ` Sylwester Nawrocki
2011-10-23  8:07                       ` Sakari Ailus
2011-10-23  8:35                         ` Sylwester Nawrocki
2011-10-23  8:44                           ` Sakari Ailus
2011-10-23  9:01                             ` Sylwester Nawrocki
2011-10-23  9:26                               ` Laurent Pinchart
2011-10-23  9:53                                 ` Sylwester Nawrocki
2011-10-20  9:45                     ` Laurent Pinchart

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