On 09/20/2012 08:49 PM, Michael Krufky wrote:
On Thu, Sep 20, 2012 at 1:47 PM, Michael Krufky <mkrufky@xxxxxxxxxxx> wrote:
On Mon, Aug 6, 2012 at 3:13 PM, Antti Palosaari <crope@xxxxxx> wrote:
On 08/06/2012 09:57 PM, Michael Krufky wrote:
On Mon, Aug 6, 2012 at 2:35 PM, Devin Heitmueller
<dheitmueller@xxxxxxxxxxxxxx> wrote:
On Mon, Aug 6, 2012 at 2:19 PM, Antti Palosaari <crope@xxxxxx> wrote:
You should understand from DVB driver model:
* attach() called only once when driver is loaded
* init() called to wake-up device
* sleep() called to sleep device
What I would like to say is that there is very big risk to shoot own leg
when doing some initialization on attach(). For example resuming from
the
suspend could cause device reset and if you rely some settings that are
done
during attach() you will likely fail as Kernel / USB-host controller has
reset your device.
See reset_resume from Kernel documentation:
Documentation/usb/power-management.txt
Be forewarned: there is a very high likelihood that this patch will
cause regressions on hybrid devices due to known race conditions
related to dvb_frontend sleeping the tuner asynchronously on close.
This is a hybrid tuner, and unless code is specifically added to the
bridge or tuner driver, going from digital to analog mode too quickly
will cause the tuner to be shutdown while it's actively in analog
mode.
(I discovered this the hard way when working on problems MythTV users
reported against the HVR-950q).
Description of race:
1. User opens DVB frontend tunes
2. User closes DVB frontend
3. User *immediately* opens V4L device using same tuner
4. User performs tuning request for analog
5. DVB frontend issues sleep() call to tuner, causing analog tuning to
fail.
This class of problem isn't seen on DVB only devices or those that
have dedicated digital tuners not shared for analog usage. And in
some cases it isn't noticed because a delay between closing the DVB
device and opening the analog devices causes the sleep() call to
happen before the analog tune (in which case you don't hit the race).
I'm certainly not against improved power management, but it will
require the race conditions to be fixed first in order to avoid
regressions.
Devin
--
Devin J. Heitmueller - Kernel Labs
http://www.kernellabs.com
Devin's right. I'm sorry, please *don't* merge the patch, Mauro.
Antti, you should just call sleep from your driver after attach(), as
I had previously suggested. We can revisit this some time in the
future after we can be sure that these race conditions wont occur.
OK, maybe it is safer then. I have no any hybrid hardware to test...
Anyhow, I wonder how many years it will take to resolve that V4L2/DVB API
hybrid usage pŕoblem. I ran thinking that recently when looked how to
implement DVB SDR for V4L2 API... I could guess problem will not disappear
near future even analog TV disappears, because there is surely coming new
nasty features which spreads over both V4L2 and DVB APIs.
Guys,
Please take another look at this branch and test if possible -- I
pushed an additional patch that takes Devin's concerns into account.
After applying these patches, the tda18271 driver will behave as it
always has, but it will sleep the tuner after attaching the first
instance. If there is only one instance, then this works exactly as
Antti desires. If there are more instances, then the tuner will only
be woken up again during attach if the tda18271_need_cal_on_startup()
returns non-zero. The driver does not attempt to re-sleep the
hardware again during successive attach() calls.
I have not tested this yet myself, but I believe it resolves the
matter -- please comment.
Regards,
Mike Krufky
...in case the URL got lost:
The following changes since commit 0c7d5a6da75caecc677be1fda207b7578936770d:
Linux 3.5-rc5 (2012-07-03 22:57:41 +0300)
are available in the git repository at:
git://linuxtv.org/mkrufky/tuners tda18271
for you to fetch changes up to 4e46c5d1bbb920165fecfe7de18b2c01d9787230:
tda18271: make 'low-power standby mode after attach' multi-instance
safe (2012-09-20 13:34:29 -0400)
----------------------------------------------------------------
Michael Krufky (2):
tda18271: enter low-power standby mode at the end of tda18271_attach()
tda18271: make 'low-power standby mode after attach' multi-instance safe
drivers/media/common/tuners/tda18271-fe.c | 4 ++++
1 file changed, 4 insertions(+)
Best regards,
Mike
Tested-by: Antti Palosaari <crope@xxxxxx>
Tested with PCTV 290e, but I cannot test multi-instance. I tried to plug
PCTV 520e as a second stick, but it crashed as there is now that DRX-K
firmware download problem....
regards
Antti
--
http://palosaari.fi/
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
