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/
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to