Re: [RFCv2 PATCH 0/5] tuner-core: fix s_std and s_tuner
Em 06-07-2011 16:59, Marko Ristola escreveu: 06.07.2011 21:17, Devin Heitmueller kirjoitti: On Wed, Jul 6, 2011 at 1:53 PM, Marko Ristola marko.rist...@kolumbus.fi wrote: All that said, I believe that you are correct in that the business logic needs to ultimately be decided by the bridge driver, rather than having the dvb/tuner core blindly calling the sleep routines against the tuner and demod drivers without a full understanding of what impact it has on the board as a whole. You wrote it nicely and compactly. What do you think about tracking coarse last busy time rather than figuring out accurate idle time? dvb_frontend.c and V4L side would just poll the device: bridge-wake(). wake() will just store current busy timestamp to the bridge device with coarse accuracy, if subdevices are already at active state. If subdevices are powered off, it will first power them on and resume them, and then store busy timestamp. Bridge device would have a delayed task: Check after 3 minutes: If I haven't been busy for three minutes, I'll go to sleep. I'll suspend the subdevices and power them off. The delayed task would refresh itself: check again after last awake time + 3 minutes. Delayed task could be further developed to support multiple suspend states. There is still an issue: Devices that support FM radio may stay closed, but with radio powered on. This is supported since the beginning by radio application (part of xawtv package). If the device is on radio mode, the only reliable way to power the device off is if the device is muted. IMO, the proper solution is to provide a core resource locking mechanism, that will keep track about what device resources are in usage (tuner, analog audio/video demods, digital demods, sec, radio reception, i2c buses, etc), and some mechanisms like the above that will power the subdevice off when it is not being used for a given amount of time (a Kconfig option can be added so set the time to X minutes or to disable such mechanism, in order to preserve back compatibility). Having a core mechanism helps to assure that it will be properly implemented on all places. This locking mechanism will be controlled by the bridge driver. It is easy to say, but it may be hard to implement, as it may require some changes at both the V4L/DVB core and at the drivers. One special case for the locking mechanisms that may or may not be covered by such core resource locking is the access to the I2C buses. Currently, the DVB, V4L and remote controller stacks may try to use resources behind I2C at the same time. The I2C core has a locking schema, but it works only if a transaction is atomically commanded. So, if it requires multiple I2C transfers, all of them need to be grouped into one i2c xfer call. Complex operations like firmware transfers are protected by the I2C locking, so one driver might be generating RC polling events while a firmware is being transferring, causing it to fail. Regards, Mauro -- 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
Re: [RFCv2 PATCH 0/5] tuner-core: fix s_std and s_tuner
Hi. IMO, your thoughts about core resource locking mechanism sound good. I say here some thoughts and examples how the resource locking mechanism might work. IMHO, It's good enough now if we are heading to a solution. At least I would not alone find a proper concept. 07.07.2011 18:14, Mauro Carvalho Chehab kirjoitti: Em 06-07-2011 16:59, Marko Ristola escreveu: 06.07.2011 21:17, Devin Heitmueller kirjoitti: On Wed, Jul 6, 2011 at 1:53 PM, Marko Ristola marko.rist...@kolumbus.fi wrote: IMO, the proper solution is to provide a core resource locking mechanism, that will keep track about what device resources are in usage (tuner, analog audio/video demods, digital demods, sec, radio reception, i2c buses, etc), and some mechanisms like the above that will power the subdevice off when it is not being used for a given amount of time (a Kconfig option can be added so set the time to X minutes or to disable such mechanism, in order to preserve back compatibility). One special case for the locking mechanisms that may or may not be covered by such core resource locking is the access to the I2C buses. Currently, the DVB, V4L and remote controller stacks may try to use resources behind I2C at the same time. The I2C core has a locking schema, but it works only if a transaction is atomically commanded. So, if it requires multiple I2C transfers, all of them need to be grouped into one i2c xfer call. Complex operations like firmware transfers are protected by the I2C locking, so one driver might be generating RC polling events while a firmware is being transferring, causing it to fail. A generic locking schema could have shared/exclusive locks by name (device name, pointer ?). A generic resource set of named locks could contain these locks. Firmware load would take an exclusive lock on I2C master for the whole transfer. RC polling would take a shared lock on I2C master and an exclusive lock on RC UART device. Or if there is no need to share I2C device, RC polling would just take exclusive lock on I2C Master. If I2C bus must be quiesced for 10ms after frontend's tuning action, I2C master exclusive lock could be held 10ms after last I2C transfer. i2c/bridge1 state should be protected if the frontend is behind an I2C bridge. The existing I2C xfer mutex would be as it is now: it is a lower level lock, no regressions to come. Taking a shared lock of tuner_power_switch would mark that the device must not be turned off. While holding the shared lock, no deferred watch would be needed. While releasing tuner_power_switch lock, usage timestamp on that name should be updated. If there are no more tuner_power_switch holders, delayed task could be activated and run after 3 minutes to power the device off if needed. Bridge drivers that don't have full runtime power saving support, would not introduce a callback which a delayed task would run to turn power off / on. Regards, Mauro IMO, suspend / resume code must be a separate thing. We might want to suspend the laptop while watching a DVB channel. We don't want to have runtime power management active while watching a DVB channel. Suspend quiesces the device possibly in one phase. It needs to have the device in a good state before suspending: take an exclusive I2C Master lock before going to suspend. While resuming, release I2C Master lock. So even though these details are incomplete, suspend/resume could perhaps be integrated with the generic advanced locking scheme. Regards, Marko -- 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
Re: [RFCv2 PATCH 0/5] tuner-core: fix s_std and s_tuner
Hi. I think that you could reuse lots of code with smart suspend / resume. What do you think about this DVB power saving case (about the concept, don't look at details, please): - One device has responsibility to do the power off when it can be done (mantis_core.ko) - In my case there is only one frontend tda10021.ko to take care of. - dvb_frontend.c would call fe-sleep(fe). The callback goes into mantis_core.ko. - mantis_core.ko will traverse all devices on it's responsibility, all (tda10021.ko) are idle. = suspend tda10021.ko by calling tda10021-sleep() and do additional power off things. - When dvb_frontend.c wants tuner to be woken up, mantis_core.ko does hardware resets and power on first and then resumes tda10021-init(fe). I implemented something that worked a few years ago with suspend / resume. In mantis_dvb.c's driver probe function I modified tuner_ops to enable these runtime powersaving features: + mantis-fe-ops.tuner_ops.sleep = mantis_dvb_tuner_sleep; + mantis-fe-ops.tuner_ops.init = mantis_dvb_tuner_init; That way mantis_core.ko had all needed details to do any advanced power savings I wanted. Suspend / resume worked well: During resume there was only a glitch at the picture and sound and the TV channel watching continued. tda10021 (was cu1216 at that time) restored the original TV channel. It took DVB FE_LOCK during resume. Suspended DMA transfer was recovered before returning into userspace. So I think that you need a single device (mantis_core.ko) that can see the whole picture, in what states the subdevices are: can you touch the power button?. With DVB this is easy because dvb_frontend.c tells when a frontend is idle and when it is not. The similar idea of some kind of watchdog that is able to track when a single device (frontend) is used and when it is not used, would be sufficient. The topmost driver (mantis_core.ko in my case) would then be responsible to track multiple frontends (subdevices), if they all are idle or not, with suitable mutex protection. Then this driver could easilly suspend/resume these subdevices and press power switch when necessary. So the clash between DVB and V4L devices would be solved: Both DVB and V4L calls a (different) sleep() function on mantis_core.ko mantis_core.ko will turn power off when both frontends are sleeping. If only one sleeps, the one can be put to sleep or suspended, but power button can't be touched. What do you think? I did this easy case mantis_core.ko solution in about Summer 2007. It needs a rewrite and testing, if I take it from the dust. Regards, Marko Ristola 16.06.2011 15:51, Devin Heitmueller wrote: On Thu, Jun 16, 2011 at 7:14 AM, Mauro Carvalho Chehab mche...@redhat.com wrote: One possible logic that would solve the scripting would be to use a watchdog to monitor ioctl activities. If not used for a while, it could send a s_power to put the device to sleep, but this may not solve all our problems. So, I agree with Devin: we need to add an option to explicitly control the power management logic of the device, having 3 modes of operation: POWER_AUTO - use the watchdogs to poweroff POWER_ON - explicitly powers on whatever subdevices are needed in order to make the V4L ready to stream; POWER_OFF - Put all subdevices to power-off if they support it. After implementing such logic, and keeping the default as POWER_ON, we may announce that the default will change to POWER_AUTO, and give some time for userspace apps/scripts that need to use a different mode to change their behaviour. That means that, for example, radio -qf will need to change to POWER_ON mode, and radio -m should call POWER_OFF. I've considered this idea before, and it's not bad in theory. The one thing you will definitely have to watch out for is causing a race between DVB and V4L for hybrid tuners. In other words, you can have a user switching from analog to digital and you don't want the tuner to get powered down a few seconds after they started streaming video from DVB. Any such solution would have to take the above into account. We've got a history of race conditions like this and I definitely don't want to see a new one introduced. Devin -- 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
Re: [RFCv2 PATCH 0/5] tuner-core: fix s_std and s_tuner
On Wed, Jul 6, 2011 at 1:53 PM, Marko Ristola marko.rist...@kolumbus.fi wrote: Hi. I think that you could reuse lots of code with smart suspend / resume. What do you think about this DVB power saving case (about the concept, don't look at details, please): - One device has responsibility to do the power off when it can be done (mantis_core.ko) - In my case there is only one frontend tda10021.ko to take care of. - dvb_frontend.c would call fe-sleep(fe). The callback goes into mantis_core.ko. - mantis_core.ko will traverse all devices on it's responsibility, all (tda10021.ko) are idle. = suspend tda10021.ko by calling tda10021-sleep() and do additional power off things. - When dvb_frontend.c wants tuner to be woken up, mantis_core.ko does hardware resets and power on first and then resumes tda10021-init(fe). I implemented something that worked a few years ago with suspend / resume. In mantis_dvb.c's driver probe function I modified tuner_ops to enable these runtime powersaving features: + mantis-fe-ops.tuner_ops.sleep = mantis_dvb_tuner_sleep; + mantis-fe-ops.tuner_ops.init = mantis_dvb_tuner_init; That way mantis_core.ko had all needed details to do any advanced power savings I wanted. Suspend / resume worked well: During resume there was only a glitch at the picture and sound and the TV channel watching continued. tda10021 (was cu1216 at that time) restored the original TV channel. It took DVB FE_LOCK during resume. Suspended DMA transfer was recovered before returning into userspace. So I think that you need a single device (mantis_core.ko) that can see the whole picture, in what states the subdevices are: can you touch the power button?. With DVB this is easy because dvb_frontend.c tells when a frontend is idle and when it is not. The similar idea of some kind of watchdog that is able to track when a single device (frontend) is used and when it is not used, would be sufficient. The topmost driver (mantis_core.ko in my case) would then be responsible to track multiple frontends (subdevices), if they all are idle or not, with suitable mutex protection. Then this driver could easilly suspend/resume these subdevices and press power switch when necessary. So the clash between DVB and V4L devices would be solved: Both DVB and V4L calls a (different) sleep() function on mantis_core.ko mantis_core.ko will turn power off when both frontends are sleeping. If only one sleeps, the one can be put to sleep or suspended, but power button can't be touched. What do you think? I did this easy case mantis_core.ko solution in about Summer 2007. It needs a rewrite and testing, if I take it from the dust. Regards, Marko Ristola Hi Marko, This is one of those ideas that sounds good in theory but in practice getting it to work with all the different types of boards is really a challenge. It would need to take into account devices with multiple frontends, shared tuners between V4L and DVB, shared tuners between different DVB frontends, etc. For example, the DVB frontend call to sleep the demod is actually deferred. This exposes all sorts of fun races with applications such as MythTV which switch between DVB and V4L modes in fast succession. For example, on the HVR-950Q I debugged an issue last year where switching from DVB to V4L would close the DVB frontend device, immediately open the V4L device, start tuning the V4L device, and then a couple hundred milliseconds later the sleep call would arrive from the DVB frontend thread which would screw up the tuner state. In other words, the locking is not trivial and you need to take into account the various threads that can be running. Taking the above example, if you had deferred freeing up the device until the sleep call arrived, then the DVB close would have returned immediately, and the V4L open would have failed because the device was still in use. Also, you need to take into consideration that bringing some devices out of sleep can be a *very* time consuming operation. This is mostly due to loading of firmware, which on some devices can take upwards of ten seconds due to large blobs and slow i2c busses. Hence if you have too granular a power management strategy you end up with a situation where you are continuously calling a resume routine which takes ten seconds. This adds up quickly if for example you call v4l2-ctl half a dozen times before starting streaming. All that said, I believe that you are correct in that the business logic needs to ultimately be decided by the bridge driver, rather than having the dvb/tuner core blindly calling the sleep routines against the tuner and demod drivers without a full understanding of what impact it has on the board as a whole. Cheers, Devin -- Devin J. Heitmueller - Kernel Labs http://www.kernellabs.com -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a
Re: [RFCv2 PATCH 0/5] tuner-core: fix s_std and s_tuner
06.07.2011 21:17, Devin Heitmueller kirjoitti: On Wed, Jul 6, 2011 at 1:53 PM, Marko Ristola marko.rist...@kolumbus.fi wrote: All that said, I believe that you are correct in that the business logic needs to ultimately be decided by the bridge driver, rather than having the dvb/tuner core blindly calling the sleep routines against the tuner and demod drivers without a full understanding of what impact it has on the board as a whole. You wrote it nicely and compactly. What do you think about tracking coarse last busy time rather than figuring out accurate idle time? dvb_frontend.c and V4L side would just poll the device: bridge-wake(). wake() will just store current busy timestamp to the bridge device with coarse accuracy, if subdevices are already at active state. If subdevices are powered off, it will first power them on and resume them, and then store busy timestamp. Bridge device would have a delayed task: Check after 3 minutes: If I haven't been busy for three minutes, I'll go to sleep. I'll suspend the subdevices and power them off. The delayed task would refresh itself: check again after last awake time + 3 minutes. Delayed task could be further developed to support multiple suspend states. Cheers, Devin Marko -- 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
Re: [RFCv2 PATCH 0/5] tuner-core: fix s_std and s_tuner
On Wednesday, June 15, 2011 22:49:39 Devin Heitmueller wrote: On Wed, Jun 15, 2011 at 4:37 PM, Hans Verkuil hverk...@xs4all.nl wrote: But the driver has that information, so it should act accordingly. So on first open you can check whether the current input has a tuner and power on the tuner in that case. On S_INPUT you can also poweron/off accordingly (bit iffy against the spec, though). So in that case the first use case would actually work. It does require that tuner-core.c supports s_power(1), of course. This will get messy, and is almost certain to get screwed up and cause regressions at least on some devices. I don't see why this should be messy. Anyway, this is all theoretical as long as tuner-core doesn't support s_power(1). Let's get that in first. BTW, I noticed in tuner-core.c that the g_tuner op doesn't wake-up the tuner when called. I think it should be added, even though most (all?) tuners will need time before they can return anything sensible. Bear in mind that some tuners can take several seconds to load firmware when powered up. You don't want a situation where the tuner is reloading firmware continuously, the net result being that calls to v4l2-ctl that used to take milliseconds now take multiple seconds. Yes, but calling VIDIOC_G_TUNER is a good time to wake up the tuner :-) BTW2: it's not a good idea to just broadcast s_power to all subdevs. That should be done to the tuner(s) only since other subdevs might also implement s_power. For now it's pretty much just tuners and some sensors, though. You know, this really needs to be a standardized module option and/or sysfs entry: 'always on', 'wake up on first open', 'wake up on first use'. That would definitely be useful, but it shouldn't be a module option since you can have multiple devices using the same module. Of course, I forgot about that. It really should be an addition to the V4L API. This would actually for once be a good use of sysfs. Regards, Hans -- 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
Re: [RFCv2 PATCH 0/5] tuner-core: fix s_std and s_tuner
Em 16-06-2011 03:21, Hans Verkuil escreveu: On Wednesday, June 15, 2011 22:49:39 Devin Heitmueller wrote: On Wed, Jun 15, 2011 at 4:37 PM, Hans Verkuil hverk...@xs4all.nl wrote: But the driver has that information, so it should act accordingly. So on first open you can check whether the current input has a tuner and power on the tuner in that case. On S_INPUT you can also poweron/off accordingly (bit iffy against the spec, though). So in that case the first use case would actually work. It does require that tuner-core.c supports s_power(1), of course. This will get messy, and is almost certain to get screwed up and cause regressions at least on some devices. I don't see why this should be messy. Anyway, this is all theoretical as long as tuner-core doesn't support s_power(1). Let's get that in first. Adding s_power to tuner-core is the easiest part. The hardest one is to decide what would be the proper behaviour. See bellow. BTW, I noticed in tuner-core.c that the g_tuner op doesn't wake-up the tuner when called. I think it should be added, even though most (all?) tuners will need time before they can return anything sensible. Bear in mind that some tuners can take several seconds to load firmware when powered up. You don't want a situation where the tuner is reloading firmware continuously, the net result being that calls to v4l2-ctl that used to take milliseconds now take multiple seconds. The question is not when to wake up the tuner, but when to put it to sleep. Really, the big issue is on USB devices, where we don't want to spend lots of power while the device is not active. Some devices even become hot when the tuner is not sleeping. As Devin pointed, some devices like tm6000 take about 30-45 seconds for loading a firmware (ok, it is a broken design. We should not take it as a good example). Currently, there's no way to know when a radio device is being used or not. Even for video, scripts that call v4l-ctl or v4l2-ctl and then start some userspace application are there for years. We have even an example for that at the v4l-utils: contrib/v4l_rec.pl. One possible logic that would solve the scripting would be to use a watchdog to monitor ioctl activities. If not used for a while, it could send a s_power to put the device to sleep, but this may not solve all our problems. So, I agree with Devin: we need to add an option to explicitly control the power management logic of the device, having 3 modes of operation: POWER_AUTO - use the watchdogs to poweroff POWER_ON - explicitly powers on whatever subdevices are needed in order to make the V4L ready to stream; POWER_OFF - Put all subdevices to power-off if they support it. After implementing such logic, and keeping the default as POWER_ON, we may announce that the default will change to POWER_AUTO, and give some time for userspace apps/scripts that need to use a different mode to change their behaviour. That means that, for example, radio -qf will need to change to POWER_ON mode, and radio -m should call POWER_OFF. It would be good if the API would also provide a way to warn userspace that a given device supports it or not (maybe at VIDIOC_QUERYCTL?). I think that such API can be implemented as a new V4L control. Yes, but calling VIDIOC_G_TUNER is a good time to wake up the tuner :-) Not really. Several popular devices load firmware based on the standard (the ones based on xc2028/xc3028/xc4000). So, before sending any tuner command, a VIDIOC_S_STD is needed. BTW2: it's not a good idea to just broadcast s_power to all subdevs. That should be done to the tuner(s) only since other subdevs might also implement s_power. For now it's pretty much just tuners and some sensors, though. You know, this really needs to be a standardized module option and/or sysfs entry: 'always on', 'wake up on first open', 'wake up on first use'. That would definitely be useful, but it shouldn't be a module option since you can have multiple devices using the same module. Of course, I forgot about that. It really should be an addition to the V4L API. This would actually for once be a good use of sysfs. Regards, Hans -- 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
Re: [RFCv2 PATCH 0/5] tuner-core: fix s_std and s_tuner
On Thu, Jun 16, 2011 at 7:14 AM, Mauro Carvalho Chehab mche...@redhat.com wrote: One possible logic that would solve the scripting would be to use a watchdog to monitor ioctl activities. If not used for a while, it could send a s_power to put the device to sleep, but this may not solve all our problems. So, I agree with Devin: we need to add an option to explicitly control the power management logic of the device, having 3 modes of operation: POWER_AUTO - use the watchdogs to poweroff POWER_ON - explicitly powers on whatever subdevices are needed in order to make the V4L ready to stream; POWER_OFF - Put all subdevices to power-off if they support it. After implementing such logic, and keeping the default as POWER_ON, we may announce that the default will change to POWER_AUTO, and give some time for userspace apps/scripts that need to use a different mode to change their behaviour. That means that, for example, radio -qf will need to change to POWER_ON mode, and radio -m should call POWER_OFF. I've considered this idea before, and it's not bad in theory. The one thing you will definitely have to watch out for is causing a race between DVB and V4L for hybrid tuners. In other words, you can have a user switching from analog to digital and you don't want the tuner to get powered down a few seconds after they started streaming video from DVB. Any such solution would have to take the above into account. We've got a history of race conditions like this and I definitely don't want to see a new one introduced. Devin -- Devin J. Heitmueller - Kernel Labs http://www.kernellabs.com -- 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
Re: [RFCv2 PATCH 0/5] tuner-core: fix s_std and s_tuner
On Saturday, June 11, 2011 19:08:04 Devin Heitmueller wrote:
On Sat, Jun 11, 2011 at 1:02 PM, Hans Verkuil hverk...@xs4all.nl wrote:
OK, but how do you get it into standby in the first place? (I must be
missing
something here...)
The tuner core puts the chip into standby when the last V4L filehandle
is closed. Yes, I realize this violates the V4L spec since you should
be able to make multiple calls with something like v4l2-ctl, but
nobody has ever come up with a better mechanism for knowing when to
put the device to sleep.
Why would that violate the spec? If the last filehandle is closed, then
you can safely poweroff the tuner. The only exception is when you have a radio
tuner whose audio output is hooked up to some line-in: there you can't power off
the tuner.
We've been forced to choose between the purist perspective, which is
properly preserving state, never powering down the tuner and sucking
up 500ma on the USB port when not using the tuner, versus powering
down the tuner when the last party closes the filehandle, which
preserves power but breaks v4l2 conformance and in some cases is
highly noticeable with tuners that require firmware to be reloaded
when being powered back up.
Seems fine to me. What isn't fine is that a driver like e.g. em28xx powers off
the tuner but doesn't power it on again on the next open. It won't do that
until the first S_FREQUENCY/S_TUNER/S_STD call.
Devin, Mauro, is there anything wrong with adding support for s_power(1) and
calling it in em28xx? A similar power up would be needed in cx23885, au0828,
cx88 and cx231xx.
Mauro, if you agree with this patch, then I'll add it to the tuner patch series.
Tested with em28xx.
Regards,
Hans
diff --git a/drivers/media/video/em28xx/em28xx-video.c
b/drivers/media/video/em28xx/em28xx-video.c
index 7b6461d..6f3f51b 100644
--- a/drivers/media/video/em28xx/em28xx-video.c
+++ b/drivers/media/video/em28xx/em28xx-video.c
@@ -2111,6 +2111,7 @@ static int em28xx_v4l2_open(struct file *filp)
em28xx_wake_i2c(dev);
}
+ v4l2_device_call_all(dev-v4l2_dev, 0, core, s_power, 1);
if (fh-radio) {
em28xx_videodbg(video_open: setting radio device\n);
v4l2_device_call_all(dev-v4l2_dev, 0, tuner, s_radio);
diff --git a/drivers/media/video/tuner-core.c b/drivers/media/video/tuner-core.c
index 9b0d833..9006c9a 100644
--- a/drivers/media/video/tuner-core.c
+++ b/drivers/media/video/tuner-core.c
@@ -1057,16 +1057,20 @@ static int tuner_s_radio(struct v4l2_subdev *sd)
/**
* tuner_s_power - controls the power state of the tuner
* @sd: pointer to struct v4l2_subdev
- * @on: a zero value puts the tuner to sleep
+ * @on: a zero value puts the tuner to sleep, non-zero wakes it up
*/
static int tuner_s_power(struct v4l2_subdev *sd, int on)
{
struct tuner *t = to_tuner(sd);
struct analog_demod_ops *analog_ops = t-fe.ops.analog_ops;
- /* FIXME: Why this function don't wake the tuner if on != 0 ? */
- if (on)
+ if (on) {
+ if (t-standby set_mode(t, t-mode) == 0) {
+ tuner_dbg(Waking up tuner\n);
+ set_freq(t, 0);
+ }
return 0;
+ }
tuner_dbg(Putting tuner to sleep\n);
t-standby = true;
--
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
Re: [RFCv2 PATCH 0/5] tuner-core: fix s_std and s_tuner
On Wed, Jun 15, 2011 at 3:55 PM, Hans Verkuil hverk...@xs4all.nl wrote: Why would that violate the spec? If the last filehandle is closed, then you can safely poweroff the tuner. The only exception is when you have a radio tuner whose audio output is hooked up to some line-in: there you can't power off the tuner. The use case that some expect to work is: v4l2-ctl set standard v4l2-ctl set frequency cat /dev/video0 out.mpg By powering off the tuner after v4l2-ctl closes the device node, the cat won't work as expected because the tuner will be powered down. We've been forced to choose between the purist perspective, which is properly preserving state, never powering down the tuner and sucking up 500ma on the USB port when not using the tuner, versus powering down the tuner when the last party closes the filehandle, which preserves power but breaks v4l2 conformance and in some cases is highly noticeable with tuners that require firmware to be reloaded when being powered back up. Seems fine to me. What isn't fine is that a driver like e.g. em28xx powers off the tuner but doesn't power it on again on the next open. It won't do that until the first S_FREQUENCY/S_TUNER/S_STD call. You don't want to power up the tuner unless you know the user intends to use it. For example, you don't want to power up the tuner if the user intends to capture on composite/s-video input (as this consumes considerably more power). Devin -- Devin J. Heitmueller - Kernel Labs http://www.kernellabs.com -- 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
Re: [RFCv2 PATCH 0/5] tuner-core: fix s_std and s_tuner
On Wednesday, June 15, 2011 22:09:45 Devin Heitmueller wrote: On Wed, Jun 15, 2011 at 3:55 PM, Hans Verkuil hverk...@xs4all.nl wrote: Why would that violate the spec? If the last filehandle is closed, then you can safely poweroff the tuner. The only exception is when you have a radio tuner whose audio output is hooked up to some line-in: there you can't power off the tuner. The use case that some expect to work is: v4l2-ctl set standard v4l2-ctl set frequency cat /dev/video0 out.mpg By powering off the tuner after v4l2-ctl closes the device node, the cat won't work as expected because the tuner will be powered down. We've been forced to choose between the purist perspective, which is properly preserving state, never powering down the tuner and sucking up 500ma on the USB port when not using the tuner, versus powering down the tuner when the last party closes the filehandle, which preserves power but breaks v4l2 conformance and in some cases is highly noticeable with tuners that require firmware to be reloaded when being powered back up. Seems fine to me. What isn't fine is that a driver like e.g. em28xx powers off the tuner but doesn't power it on again on the next open. It won't do that until the first S_FREQUENCY/S_TUNER/S_STD call. You don't want to power up the tuner unless you know the user intends to use it. For example, you don't want to power up the tuner if the user intends to capture on composite/s-video input (as this consumes considerably more power). But the driver has that information, so it should act accordingly. So on first open you can check whether the current input has a tuner and power on the tuner in that case. On S_INPUT you can also poweron/off accordingly (bit iffy against the spec, though). So in that case the first use case would actually work. It does require that tuner-core.c supports s_power(1), of course. BTW, I noticed in tuner-core.c that the g_tuner op doesn't wake-up the tuner when called. I think it should be added, even though most (all?) tuners will need time before they can return anything sensible. BTW2: it's not a good idea to just broadcast s_power to all subdevs. That should be done to the tuner(s) only since other subdevs might also implement s_power. For now it's pretty much just tuners and some sensors, though. You know, this really needs to be a standardized module option and/or sysfs entry: 'always on', 'wake up on first open', 'wake up on first use'. Regards, Hans -- 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
Re: [RFCv2 PATCH 0/5] tuner-core: fix s_std and s_tuner
On Wed, Jun 15, 2011 at 4:37 PM, Hans Verkuil hverk...@xs4all.nl wrote: But the driver has that information, so it should act accordingly. So on first open you can check whether the current input has a tuner and power on the tuner in that case. On S_INPUT you can also poweron/off accordingly (bit iffy against the spec, though). So in that case the first use case would actually work. It does require that tuner-core.c supports s_power(1), of course. This will get messy, and is almost certain to get screwed up and cause regressions at least on some devices. BTW, I noticed in tuner-core.c that the g_tuner op doesn't wake-up the tuner when called. I think it should be added, even though most (all?) tuners will need time before they can return anything sensible. Bear in mind that some tuners can take several seconds to load firmware when powered up. You don't want a situation where the tuner is reloading firmware continuously, the net result being that calls to v4l2-ctl that used to take milliseconds now take multiple seconds. BTW2: it's not a good idea to just broadcast s_power to all subdevs. That should be done to the tuner(s) only since other subdevs might also implement s_power. For now it's pretty much just tuners and some sensors, though. You know, this really needs to be a standardized module option and/or sysfs entry: 'always on', 'wake up on first open', 'wake up on first use'. That would definitely be useful, but it shouldn't be a module option since you can have multiple devices using the same module. It really should be an addition to the V4L API. Devin -- Devin J. Heitmueller - Kernel Labs http://www.kernellabs.com -- 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
Re: [RFCv2 PATCH 0/5] tuner-core: fix s_std and s_tuner
On Sat, Jun 11, 2011 at 11:05 AM, Hans Verkuil hverk...@xs4all.nl wrote: Second version of this patch series. It's the same as RFCv1, except that I dropped the g_frequency and g_tuner/s_tuner patches (patch 3, 6 and 7 in the original patch series) because I need to think more on those, and I added a new fix for tuner_resume which was broken as well. Hi Hans, I appreciate your taking the time to refactor this code (no doubt it really needed it). All that I ask is that you please actually *try* the resulting patches with VLC and a tuner that supports standby in order to ensure that it didn't cause any regressions. This stuff was brittle to begin with, and there are lots of opportunities for obscure/unexpected effects resulting from what appear to be sane changes. The last series of patches that went in were in response to this stuff being very broken, and I would hate to see a regression in existing applications after we finally got it working. Thanks, Devin -- Devin J. Heitmueller - Kernel Labs http://www.kernellabs.com -- 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
Re: [RFCv2 PATCH 0/5] tuner-core: fix s_std and s_tuner
On Saturday, June 11, 2011 17:32:10 Devin Heitmueller wrote: On Sat, Jun 11, 2011 at 11:05 AM, Hans Verkuil hverk...@xs4all.nl wrote: Second version of this patch series. It's the same as RFCv1, except that I dropped the g_frequency and g_tuner/s_tuner patches (patch 3, 6 and 7 in the original patch series) because I need to think more on those, and I added a new fix for tuner_resume which was broken as well. Hi Hans, I appreciate your taking the time to refactor this code (no doubt it really needed it). All that I ask is that you please actually *try* the resulting patches with VLC and a tuner that supports standby in order to ensure that it didn't cause any regressions. That's easier said than done. I don't think I have tuners of that type. Do you happen to know not-too-expensive cards that you can buy that have this sort of tuners? It may be useful to be able to test this myself. This stuff was brittle to begin with, and there are lots of opportunities for obscure/unexpected effects resulting from what appear to be sane changes. The last series of patches that went in were in response to this stuff being very broken, and I would hate to see a regression in existing applications after we finally got it working. Yeah, it seems that whenever you touch this tuner code something breaks for at least one card. There is so much legacy here... Regards, Hans -- 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
Re: [RFCv2 PATCH 0/5] tuner-core: fix s_std and s_tuner
Hans Verkuil hverk...@xs4all.nl wrote: On Saturday, June 11, 2011 17:32:10 Devin Heitmueller wrote: On Sat, Jun 11, 2011 at 11:05 AM, Hans Verkuil hverk...@xs4all.nl wrote: Second version of this patch series. It's the same as RFCv1, except that I dropped the g_frequency and g_tuner/s_tuner patches (patch 3, 6 and 7 in the original patch series) because I need to think more on those, and I added a new fix for tuner_resume which was broken as well. Hi Hans, I appreciate your taking the time to refactor this code (no doubt it really needed it). All that I ask is that you please actually *try* the resulting patches with VLC and a tuner that supports standby in order to ensure that it didn't cause any regressions. That's easier said than done. I don't think I have tuners of that type. Do you happen to know not-too-expensive cards that you can buy that have this sort of tuners? It may be useful to be able to test this myself. This stuff was brittle to begin with, and there are lots of opportunities for obscure/unexpected effects resulting from what appear to be sane changes. The last series of patches that went in were in response to this stuff being very broken, and I would hate to see a regression in existing applications after we finally got it working. Yeah, it seems that whenever you touch this tuner code something breaks for at least one card. There is so much legacy here... Regards, Hans -- 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 Devin, I think I have a Gotview or compro card with an xc2028. Is that tuner capable of standby? Would the cx18 or ivtv driver need to actively support using stand by? -Andy -- 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
Re: [RFCv2 PATCH 0/5] tuner-core: fix s_std and s_tuner
On Sat, Jun 11, 2011 at 12:06 PM, Andy Walls awa...@md.metrocast.net wrote: Devin, I think I have a Gotview or compro card with an xc2028. Is that tuner capable of standby? Would the cx18 or ivtv driver need to actively support using stand by? An xc2028/xc3028 should be fine, as that does support standby. The problems we saw with VLC were related to calls like G_TUNER returning prematurely if the device was in standby, leaving the returned structure populated with garbage. FWIW, I don't dispute your assertion that Hans found legitimate bugs - just that we need to be careful to not cause regressions in the cases that the last round of bug fixes addressed. Devin -- Devin J. Heitmueller - Kernel Labs http://www.kernellabs.com -- 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
Re: [RFCv2 PATCH 0/5] tuner-core: fix s_std and s_tuner
On Sat, Jun 11, 2011 at 11:53 AM, Hans Verkuil hverk...@xs4all.nl wrote: Do you happen to know not-too-expensive cards that you can buy that have this sort of tuners? It may be useful to be able to test this myself. Anything with an xc3028 or xc5000 would have this issue. I don't really keep close track of current shipping DVB products, but the 3028 was definitely very chip in products from a couple of years ago. Devin -- Devin J. Heitmueller - Kernel Labs http://www.kernellabs.com -- 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
Re: [RFCv2 PATCH 0/5] tuner-core: fix s_std and s_tuner
On Sat, Jun 11, 2011 at 1:02 PM, Hans Verkuil hverk...@xs4all.nl wrote: OK, but how do you get it into standby in the first place? (I must be missing something here...) The tuner core puts the chip into standby when the last V4L filehandle is closed. Yes, I realize this violates the V4L spec since you should be able to make multiple calls with something like v4l2-ctl, but nobody has ever come up with a better mechanism for knowing when to put the device to sleep. We've been forced to choose between the purist perspective, which is properly preserving state, never powering down the tuner and sucking up 500ma on the USB port when not using the tuner, versus powering down the tuner when the last party closes the filehandle, which preserves power but breaks v4l2 conformance and in some cases is highly noticeable with tuners that require firmware to be reloaded when being powered back up. Devin -- Devin J. Heitmueller - Kernel Labs http://www.kernellabs.com -- 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
