Hi Sean, On 08/04/2019 16.37, Brad Love wrote: > Hi Sean, > > > On 08/04/2019 15.55, Sean Young wrote: >> On Mon, Apr 08, 2019 at 01:14:08PM -0500, Brad Love wrote: >>> On 05/04/2019 05.29, Sean Young wrote: >>>> On Sat, Dec 29, 2018 at 11:51:16AM -0600, Brad Love wrote: >>>>> Some software reports no signal found on a frequency due to immediately >>>>> checking for lock as soon as set_params returns. This waits up 40ms for >>>>> tuning operation, then from 30-150ms (dig/analog) for signal lock before >>>>> returning from set_params and set_analog_params. >>>>> >>>>> Tuning typically completes in 20-30ms. Digital tuning will additionally >>>>> wait depending on signal characteristics. Analog tuning will wait the >>>>> full timeout in case of no signal. >>>> This looks like a workaround for broken userspace. Very possibly this >>>> is software was tested on a different frontend which does wait for tuning >>>> to complete. >>>> >>>> This is a change in uapi and will have to be done for all frontends, and >>>> documented. However I doubt such a change is acceptable. >>>> >>>> What software are you refering to? >>> Hi Sean, >>> >>> I would have to check with support to find out the various software that >>> expect when a tune command returns for that tuning operation to have >>> actually completed. In the current state when you tune si2157 it >>> immediately returns, no care of the tuning operation completion, no care >>> of the pll lock success. This is correct? Not so according to Silicon >>> Labs documentation, which suggests a brief wait. I just took a look at >>> other drivers, sampling only those with set_analog_params, all but two >>> have similar code in place to actually allow the tune operation time to >>> complete (both digital and analog) before returning to userland. The >>> other drivers just insert arbitrary time delays to accommodate the >>> operations completion time. At least with my patch I am monitoring the >>> hardware to know when the operation actually completes. >>> >>> I see in tuners (not frontends): >>> >>> mt2063 - waits up to 100ms for lock status and return >>> r820t - sleep 100ms for pll lock status and return >>> tda827x - 170-500ms wait after tune before checking status and return >>> tda8290 - sleep 100ms up to 3x while checking tune status and return >>> tuner-xc2028 - sleep for 110ms awaiting lock status and return >>> xc4000 - 10ms sleep after tune, unless debug, then 100ms and return >>> xc5000 - 20ms sleep after tune, if pll not locked, re-tune and sleep >>> again, repeat until success and return >>> >>> There's also other arbitrary sleeps peppered throughout the operations. >>> >>> Then you have si2157 that fires off the tuning commands and goes right >>> back to userland immediately, when with instrumented testing, the >>> operation takes time to complete and lock. The operation does not happen >>> instantaneously. Software that expects clocks to be locked when the >>> function returns determine this is an error. They query the tuner >>> immediately when tune returns and they output tuning failed. >>> >>> Please explain why awaiting the hardware to say the "tuning operation >>> you requested is done and clocks are locked" is not ok. If it's not ok, >>> fine, but then a lot of other drivers are currently "not ok" as well. >> If you read the dvb userspace api, there is nothing in the DTV_TUNE >> property that says it will block until a lock has been made. >> >> https://www.kernel.org/doc/html/latest/media/uapi/dvb/fe_property_parameters.html#dtv-tune >> >> The locking status can be queried using read status. >> >> https://www.kernel.org/doc/html/latest/media/uapi/dvb/fe-read-status.html#fe-read-status >> >> Using this mechanism, there is a minimum of sleeping in the kernel which helps >> interactivity. (uninterruptable) sleeping in drivers something that really >> should be avoided if at all possible. Userspace can't do anything during that >> time. > > This is not dealing with signal lock. As far as I understand the > documentation I have, this is hardware tuning completion, aka the pll's > and whatever else are properly configured and the tuning operation > itself is finished successfully. The byte read is looking for > TUNINT/ATVINT/DTVINT bits set, meaning the operation is complete. Right > now I ignore errors, which probably should be returned to userland, as > if a tune fails the userland has no way to determine this fact. This > process comes directly from a flowchart in the SL guide. > > Signal lock is determined later, using the methods you describe. No > property registers are being queried here, the success of the previous > (tuning) operation is. Hi Sean, Just noticing now that my commit message does not agree with what I'm saying. I will need to do some testing to see which is correct. I've been basing my statements here off the SL reference I have, but I wrote the commit message over a year ago probably. I'll need to do some testing, I seem to recall the timing being consistent whether a signal is found or not. I might have mis-wrote the message long ago when I was splitting this up. Regards, Brad > > >> So if you look at how dvbv5-zap finds a lock, you can see that it sets >> DTV_TUNE (amongst others) and then uses the frontend read_status() to query >> for locking until either timeout or a lock is found (not sure why it polls >> one per second though, seems a bit overly conservative). >> >> https://git.linuxtv.org/v4l-utils.git/tree/utils/dvb/dvbv5-zap.c#n494 >> >> Now, if other drivers have a sleep to wait for tuning lock in them and whether >> they should be removed, is another question. Looking at the tda8290 driver >> it needs to try various things in order to make a lock, so there is (currently) >> no way to avoid this. It would be nice if it could be changed to interruptable >> sleeps though, so that dvb userspace does not "hang" until a lock is made >> or failed. > > I indeed noticed none of the other drivers use usleep_range, which is > interruptible, correct? > > Regards, > > Brad > > >> >> Thanks, >> >> Sean
