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