diff mbox

[RFC] Need testers for s5h1409 tuning fix

Message ID 412bdbff0901212045t1287a403h57ba05cbd71d5224@mail.gmail.com (mailing list archive)
State Superseded, archived
Headers

Commit Message

Devin Heitmueller Jan. 22, 2009, 4:45 a.m. UTC 
  The attached patch significantly improves tuning lock times for all
three s5h1409 based devices I have tested with so far.  However,
because of the large number of devices affected, I would like to
solicit people with products that use the s5h1409 to test the patch
and report back any problems before it gets committed.

To test the patch, check out the latest v4l-dvb and apply the patch:

hg clone http://linuxtv.org/hg/v4l-dvb
cd v4l-dvb
patch -p1 < s5h1409_tuning_speedup.patch
make
make install
make unload
reboot

Based on the data collected thus far, this patch should address some
long-standing issues with long times to reach tuning lock and
intermittent lock failures.

Comments welcome.

Thanks,

Devin

Comments

Andy Walls Jan. 23, 2009, 5:48 p.m. UTC | #1 
On Wed, 2009-01-21 at 23:45 -0500, Devin Heitmueller wrote:
> The attached patch significantly improves tuning lock times for all
> three s5h1409 based devices I have tested with so far.  However,
> because of the large number of devices affected, I would like to
> solicit people with products that use the s5h1409 to test the patch
> and report back any problems before it gets committed.
> 
> To test the patch, check out the latest v4l-dvb and apply the patch:
> 
> hg clone http://linuxtv.org/hg/v4l-dvb
> cd v4l-dvb
> patch -p1 < s5h1409_tuning_speedup.patch
> make
> make install
> make unload
> reboot
> 
> Based on the data collected thus far, this patch should address some
> long-standing issues with long times to reach tuning lock and
> intermittent lock failures.
> 
> Comments welcome.

I will test soon, but I do have two comments by inspection.

1. The function s5h1409_softreset() is now called 3 times by
s5h1409_set_frontend(): once at entry, once by
s5h1409_enable_modulation(), and once at exit.  Surely at least one of
these is not needed, no?

2.  You've eliminated the 100 ms "settle delay" after the final
softreset.  Can something from userland turn around and call one of the
s5h1409_ops vectors and muck with registers before things "settle"?
Does it even matter?

I know a hardware reset requires a long-ish assertion time (in fact now
that I see it, I have to fix the cx18 driver hardware reset assertion
delay for this device - the current value isn't right).

Regards,
Andy

> Thanks,
> 
> Devin
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Andy Walls Jan. 23, 2009, 6:05 p.m. UTC | #2 
On Wed, 2009-01-21 at 23:45 -0500, Devin Heitmueller wrote:
> The attached patch significantly improves tuning lock times for all
> three s5h1409 based devices I have tested with so far.  However,
> because of the large number of devices affected, I would like to
> solicit people with products that use the s5h1409 to test the patch
> and report back any problems before it gets committed.
> 
> To test the patch, check out the latest v4l-dvb and apply the patch:
> 
> hg clone http://linuxtv.org/hg/v4l-dvb
> cd v4l-dvb
> patch -p1 < s5h1409_tuning_speedup.patch
> make
> make install
> make unload
> reboot
> 
> Based on the data collected thus far, this patch should address some
> long-standing issues with long times to reach tuning lock and
> intermittent lock failures.
>
> Comments welcome.

Holy cow! the thing tunes fast now!

One burst error I received seemed much more devasting to mplayer's
decoder than it usually does though.  (I guess fast tuning or relocking
may have it's disadvantages, but decoding errant streams as sanely as
possible is more a software decoder's problem.) 

Propagation conditions here today are much better than in recent days
due to weather changes (it's close to 50 F!).  I'll test tonight around
sunset and later when things get colder, to get more more data points
for what happens when burts errors occur.

But right now, it looks very good. :D

Regards,
Andy

> Thanks,
> 
> Devin


--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Devin Heitmueller Jan. 23, 2009, 7:31 p.m. UTC | #3 
On Fri, Jan 23, 2009 at 12:48 PM, Andy Walls <awalls@radix.net> wrote:
> I will test soon, but I do have two comments by inspection.
>
> 1. The function s5h1409_softreset() is now called 3 times by
> s5h1409_set_frontend(): once at entry, once by
> s5h1409_enable_modulation(), and once at exit.  Surely at least one of
> these is not needed, no?
>
> 2.  You've eliminated the 100 ms "settle delay" after the final
> softreset.  Can something from userland turn around and call one of the
> s5h1409_ops vectors and muck with registers before things "settle"?
> Does it even matter?
>
> I know a hardware reset requires a long-ish assertion time (in fact now
> that I see it, I have to fix the cx18 driver hardware reset assertion
> delay for this device - the current value isn't right).
>
> Regards,
> Andy

Just to be clear, the term "softreset" is somewhat a misnomer.  It is
*not* a software equivalent of a hardware reset.  It does not reset
any of the configuration registers.  It only resets the internal
status data that is used to determine lock status.

The 100ms settle delay should not be needed at all.  If there is a
concern about something in userland mucking with the registers, that
is something that would have to be addressed through locking.  As far
as I know, there is no known issue associated with querying the status
registers as soon as the status counters are reset.

It is possible that the softreset() before the tune may not be
necessary, but at this point my intent was to focus on the minimal
change that achieves the desired effect, and the additional
softreset() should not cause any performance or reliability impact.
Also, the change I made is consistent with the change I made for the
s5h1411 back in October (for which there have been no problems
reported).

Yes, traditionally a hardware reset needs to be asserted for a
specific period of time (varies by device).  That does not apply here
though since the chip itself is not being reset.

There is additional room for optimization, but this fix alone provides
a significant performance benefit and is low impact, so I wanted to
get it merged independent of any other fixes.

Devin
Devin Heitmueller Jan. 23, 2009, 7:36 p.m. UTC | #4 
On Fri, Jan 23, 2009 at 1:05 PM, Andy Walls <awalls@radix.net> wrote:
> Holy cow! the thing tunes fast now!
>
> One burst error I received seemed much more devasting to mplayer's
> decoder than it usually does though.  (I guess fast tuning or relocking
> may have it's disadvantages, but decoding errant streams as sanely as
> possible is more a software decoder's problem.)
>
> Propagation conditions here today are much better than in recent days
> due to weather changes (it's close to 50 F!).  I'll test tonight around
> sunset and later when things get colder, to get more more data points
> for what happens when burts errors occur.
>
> But right now, it looks very good. :D
>
> Regards,
> Andy

Glad to hear that it is working well for you.

Could you please clarify what you mean by "burst error"?

For my record keeping, could you please confirm which hardware you are
doing the testing with?  This is important since there could be an
issue with your demod/tuner combination.

It would be good if you could provide some actual data regarding
before and after the patch.  Typically I run Kaffeine from the command
line, which prints out the tuning time to stdout.  For example, here
are the times Robert saw when he tested my patch:

Before the change:
Tuning delay: 2661 ms
Tuning delay: 474 ms
Tuning delay: 472 ms
Tuning lock fail after 5000ms
Tuning delay: 2000 ms
Tuning delay: 2685 ms
Tuning delay: 475 ms

After the change:
Tuning delay: 594 ms
Tuning delay: 570 ms
Tuning delay: 574 ms
Tuning delay: 671 ms
Tuning delay: 570 ms
Tuning delay: 673 ms

If you could provide something comparable, it would be useful.

Thank you for taking the time to test.

Devin
Andy Walls Jan. 23, 2009, 8:10 p.m. UTC | #5 
On Fri, 2009-01-23 at 14:36 -0500, Devin Heitmueller wrote:
> On Fri, Jan 23, 2009 at 1:05 PM, Andy Walls <awalls@radix.net> wrote:
> > Holy cow! the thing tunes fast now!
> >
> > One burst error I received seemed much more devasting to mplayer's
> > decoder than it usually does though.  (I guess fast tuning or relocking
> > may have it's disadvantages, but decoding errant streams as sanely as
> > possible is more a software decoder's problem.)
> >
> > Propagation conditions here today are much better than in recent days
> > due to weather changes (it's close to 50 F!).  I'll test tonight around
> > sunset and later when things get colder, to get more more data points
> > for what happens when burts errors occur.
> >
> > But right now, it looks very good. :D
> >
> > Regards,
> > Andy
> 
> Glad to hear that it is working well for you.
> 
> Could you please clarify what you mean by "burst error"?

A momentary deep channel fade.  I'm located > 75 miles from the TV
broadcasters.  Lots of opportunity for weather effects or aircraft to
come between my location and the broadcast towers.

In fact, rotary wing aircraft fly by to the north of my location within
eye-sight and earshot during the day somewhat regularly during the week.
They always momentarily disrupt reception as they pass by.

Before mplayer would log one maybe two lines of errors during such an
event.  With the change in place, mplayer now logs > 24 lines worth of
errors.  I assume, that's because with the change in place the 8-VSB
demodulator is now reacting faster to the poor channel condition.

For eaxmple,  Here's what mplayer will blurt out during a burst error
with the change in place:

a52: CRC check failed!
a52: error at resampling
[mpeg2video @ 0xab61c0]invalid mb type in I Frame at 23 22  0%  0.9% 1 0 28%    
[mpeg2video @ 0xab61c0]skipped MB in I frame at 14 25
[mpeg2video @ 0xab61c0]invalid mb type in I Frame at 1 28
[mpeg2video @ 0xab61c0]ac-tex damaged at 1 29
[mpeg2video @ 0xab61c0]concealing 333 DC, 333 AC, 333 MV errors
[mpeg2video @ 0xab61c0]skipped MB in I frame at 7 2634  6%  0%  0.9% 1 0 28%    
[mpeg2video @ 0xab61c0]skipped MB in I frame at 4 6
[mpeg2video @ 0xab61c0]skipped MB in I frame at 19 10
[mpeg2video @ 0xab61c0]skipped MB in I frame at 41 24
[mpeg2video @ 0xab61c0]concealing 220 DC, 220 AC, 220 MV errors
[mpeg2video @ 0xab61c0]ac-tex damaged at 6 19635/14635  6%  0%  0.9% 1 0 28%    
[mpeg2video @ 0xab61c0]concealing 176 DC, 176 AC, 176 MV errors
[mpeg2video @ 0xab61c0]00 motion_type at 25 5637/14637  6%  0%  0.9% 1 0 28%    
[mpeg2video @ 0xab61c0]concealing 135 DC, 135 AC, 135 MV errors
[mpeg2video @ 0xab61c0]invalid mb type in B Frame at 42 27  0%  0.9% 1 0 28%    
[mpeg2video @ 0xab61c0]concealing 44 DC, 44 AC, 44 MV errors
[mpeg2video @ 0xab61c0]ac-tex damaged at 37 2639/14639  6%  0%  0.9% 1 0 28%    
[mpeg2video @ 0xab61c0]00 motion_type at 3 28
[mpeg2video @ 0xab61c0]ac-tex damaged at 0 29
[mpeg2video @ 0xab61c0]concealing 176 DC, 176 AC, 176 MV errors
[mpeg2video @ 0xab61c0]00 motion_type at 22 2740/14640  6%  0%  0.9% 1 0 28%    
[mpeg2video @ 0xab61c0]mb incr damaged
[mpeg2video @ 0xab61c0]00 motion_type at 1 27
[mpeg2video @ 0xab61c0]00 motion_type at 4 28
[mpeg2video @ 0xab61c0]ac-tex damaged at 6 29
[mpeg2video @ 0xab61c0]concealing 132 DC, 132 AC, 132 MV errors
[mpeg2video @ 0xab61c0]00 motion_type at 34 2641/14641  6%  0%  0.9% 1 0 28%    
[mpeg2video @ 0xab61c0]concealing 176 DC, 176 AC, 176 MV errors
[mpeg2video @ 0xab61c0]slice mismatch0.406 14643/14643  6%  0%  0.9% 1 0 29%    
[mpeg2video @ 0xab61c0]concealing 968 DC, 968 AC, 968 MV errors
[mpeg2video @ 0xab61c0]invalid mb type in I Frame at 28 6%  0%  0.9% 1 0 29%    
[mpeg2video @ 0xab61c0]ac-tex damaged at 24 9
[mpeg2video @ 0xab61c0]invalid mb type in I Frame at 1 27
[mpeg2video @ 0xab61c0]skipped MB in I frame at 1 28
[mpeg2video @ 0xab61c0]invalid mb type in I Frame at 1 29
[mpeg2video @ 0xab61c0]concealing 1276 DC, 1276 AC, 1276 MV errors
[mpeg2video @ 0xab61c0]skipped MB in I frame at 15 196  6%  0%  0.9% 1 0 28%    
[mpeg2video @ 0xab61c0]skipped MB in I frame at 36 22
[mpeg2video @ 0xab61c0]ac-tex damaged at 13 26
[mpeg2video @ 0xab61c0]concealing 352 DC, 352 AC, 352 MV errors


Before, I'd only get a few (2 to 6) lines upon such an event.


> For my record keeping, could you please confirm which hardware you are
> doing the testing with?  This is important since there could be an
> issue with your demod/tuner combination.

HVR-1600 MCE.  It has an MXL5005s with a CX24227 (S5H1409).

Look in

	 linux/drivers/media/video/cx18/cx18-dvb.c

to see how it's being set up.


> It would be good if you could provide some actual data regarding
> before and after the patch.Typically I run Kaffeine from the command
> line, which prints out the tuning time to stdout.  For example, here
> are the times Robert saw when he tested my patch:
> 
> Before the change:
> Tuning delay: 2661 ms
> Tuning delay: 474 ms
> Tuning delay: 472 ms
> Tuning lock fail after 5000ms
> Tuning delay: 2000 ms
> Tuning delay: 2685 ms
> Tuning delay: 475 ms
> 
> After the change:
> Tuning delay: 594 ms
> Tuning delay: 570 ms
> Tuning delay: 574 ms
> Tuning delay: 671 ms
> Tuning delay: 570 ms
> Tuning delay: 673 ms
> 
> If you could provide something comparable, it would be useful.

Will do.  Any other tools you know of offhand besides kaffine that
provide such info?


> Thank you for taking the time to test.

Your welcome.

Regards,
Andy


> Devin


--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Andy Walls Jan. 24, 2009, 1:31 a.m. UTC | #6 
On Fri, 2009-01-23 at 14:36 -0500, Devin Heitmueller wrote:

> 
> It would be good if you could provide some actual data regarding
> before and after the patch.  Typically I run Kaffeine from the command
> line, which prints out the tuning time to stdout.  For example, here
> are the times Robert saw when he tested my patch:
> 
> Before the change:
> Tuning delay: 2661 ms
> Tuning delay: 474 ms
> Tuning delay: 472 ms
> Tuning lock fail after 5000ms
> Tuning delay: 2000 ms
> Tuning delay: 2685 ms
> Tuning delay: 475 ms
> 
> After the change:
> Tuning delay: 594 ms
> Tuning delay: 570 ms
> Tuning delay: 574 ms
> Tuning delay: 671 ms
> Tuning delay: 570 ms
> Tuning delay: 673 ms
> 
> If you could provide something comparable, it would be useful.

For OTA ATSC 8-VSB with an HVR-1600 MCE:

Without the change:

$ ./tune
Commanding tune to freq 479028615 ... FE_HAS_LOCK in 1.416984 seconds.
Commanding tune to freq 551028615 ... FE_HAS_LOCK in 1.389922 seconds.
Commanding tune to freq 569028615 ... FE_HAS_LOCK in 2.783927 seconds.
Commanding tune to freq 587028615 ... FE_HAS_LOCK in 1.391952 seconds.
Commanding tune to freq 593028615 ... NO lock after 2.999655 seconds.
Commanding tune to freq 599028615 ... FE_HAS_LOCK in 1.568240 seconds.
Commanding tune to freq 605028615 ... FE_HAS_LOCK in 1.390964 seconds.
Commanding tune to freq 623028615 ... NO lock after 2.999656 seconds.
Commanding tune to freq 677028615 ... FE_HAS_LOCK in 2.963289 seconds.
Commanding tune to freq 695028615 ... NO lock after 2.999618 seconds.

With the change:

$ ./tune
Commanding tune to freq 479028615 ... FE_HAS_LOCK in 1.323542 seconds.
Commanding tune to freq 551028615 ... FE_HAS_LOCK in 1.293956 seconds.
Commanding tune to freq 569028615 ... FE_HAS_LOCK in 1.292931 seconds.
Commanding tune to freq 587028615 ... FE_HAS_LOCK in 1.292973 seconds.
Commanding tune to freq 593028615 ... FE_HAS_LOCK in 1.292920 seconds.
Commanding tune to freq 599028615 ... FE_HAS_LOCK in 1.293977 seconds.
Commanding tune to freq 605028615 ... FE_HAS_LOCK in 1.292940 seconds.
Commanding tune to freq 623028615 ... FE_HAS_LOCK in 1.292949 seconds.
Commanding tune to freq 677028615 ... FE_HAS_LOCK in 1.293948 seconds.
Commanding tune to freq 695028615 ... NO lock after 2.999659 seconds.


No lock was expected for 695 MHz in either case - it was known negative.

Since I was to lazy to get Kaffeine to work properly, I wrote my own
test app.  It is inline below so you can see how I measured the time.

Regards,
Andy

/*
 * tune.c - Measure the time to tune some hardcoded ATSC OTA channels
 * Copyright (C) 2009 Andy Walls
 *
 * This program is free software; you can redistribute it and/or modify
 * it under the terms of the GNU General Public License as published by
 * the Free Software Foundation; either version 2 of the License, or
 * (at your option) any later version.
 *
 * This program is distributed in the hope that it will be useful,
 * but WITHOUT ANY WARRANTY; without even the implied warranty of
 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 * GNU General Public License for more details.
 *
 * You should have received a copy of the GNU General Public License
 * along with this program; if not, write to the Free Software
 * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
 *
 * Author: Andy Walls <awalls@radix.net>
 *
 */
#include <stdio.h>
#include <stdlib.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <fcntl.h>
#include <sys/ioctl.h>
#include <unistd.h>
#include <sys/select.h>
#include <errno.h>
#include <sys/time.h>
#include <linux/dvb/frontend.h>

unsigned int test_freq[] = {
	479028615,
	551028615,
	569028615,
	587028615,
	593028615,
	599028615,
	605028615,
	623028615,
	677028615,
	695028615,
	0
};

int main(int argc, char *argv[])
{
	int fd, ret, i;
	struct dtv_property task[3];
	struct dtv_properties tasks;

	struct timeval tv, stv, etv;
	int n, watch;
	fd_set efds;
	struct dvb_frontend_event fev;

	fd = open("/dev/dvb/adapter0/frontend0", O_RDWR|O_NONBLOCK);
	if (fd < 0) {
		perror("open");
		exit(1);
	}

	tasks.props = task;

	/* Basic ATSC 8-VSB setup */
	task[0].cmd = DTV_DELIVERY_SYSTEM;
	task[0].u.data = SYS_ATSC;
	task[0].result = 0;
	task[1].cmd = DTV_MODULATION;
	task[1].u.data = VSB_8;
	task[1].result = 0;
	tasks.num = 2;

	ret = ioctl(fd, FE_SET_PROPERTY, &tasks);
	if (ret < 0) {
		perror("FE_SET_PROPERTY");
		close(fd);
		exit(2);
	}
	
	ret = 0;
	for(i = 0; i < tasks.num; i++) {
		if (task[i].result < 0)
			ret = task[i].result;
	}

	if (ret < 0) {
		fprintf(stderr, "Failed to set ATSC 8-VSB modulation.\n");
		close(fd);
		exit(3);
	}

	/* Tune test loop */
	task[0].cmd = DTV_FREQUENCY;
	task[1].cmd = DTV_TUNE;
	task[1].u.data = 0;

	for(i = 0; test_freq[i] != 0; i++) {
		/* Tune to next freq */
		task[0].u.data = test_freq[i];
		task[0].result = 0;
		task[1].result = 0;
		tasks.num = 2;
		printf("Commanding tune to freq %u ... ", task[0].u.data);
		gettimeofday(&stv, NULL);
		ret = ioctl(fd, FE_SET_PROPERTY, &tasks);
		if (ret < 0 || task[0].result < 0 || task[1].result < 0) {
			putchar('\n');
			perror("FE_SET_PROPERTY");
			fprintf(stderr, "failed commanding tune to freq %u\n",
				task[0].u.data);
			continue;
		}

		/* Look for a frontend LOCK */
		watch = 1;
		while (watch) {
			FD_ZERO(&efds);
			FD_SET(fd, &efds);
			tv.tv_sec = 3;
			tv.tv_usec = 0;
			n = select(fd+1, NULL, NULL, &efds, &tv);
			if (n < 0) {
				if (errno == EINTR)
					continue;
				putchar('\n');
				perror("select");
				fprintf(stderr,
					"error waiting for a frontend event\n");
				break;
			}
			if (n == 0) {
				gettimeofday(&etv, NULL);
				if (stv.tv_usec > etv.tv_usec) {
					etv.tv_sec -= 1;
					etv.tv_usec += 1000000;
				}
				etv.tv_usec -= stv.tv_usec;
				etv.tv_sec -= stv.tv_sec;
				printf("NO lock after %u.%06u seconds.\n",
					etv.tv_sec, etv.tv_usec);
				break;
			}

			/* Pull off all the frontend events */
			fev.status = (fe_status_t) 0;
			etv.tv_sec = 0; etv.tv_usec = 0;
			while (1) {
				ret = ioctl(fd, FE_GET_EVENT, &fev);
				if (ret < 0) {
					if (errno == EOVERFLOW) {
						/* overflow warning */
						continue;
					}
					if (errno == EWOULDBLOCK) {
						/* event queue is empty now */
						break;
					}
					/* bad error, stop waiting for events */
					putchar('\n');
					perror("FE_GET_EVENT");
					watch = 0;
					break;
				}

				/* Scan for the first LOCK status */
				if (!watch || !(fev.status & FE_HAS_LOCK))
					continue;

				gettimeofday(&etv, NULL);
				if (stv.tv_usec > etv.tv_usec) {
					etv.tv_sec -= 1;
					etv.tv_usec += 1000000;
				}
				etv.tv_usec -= stv.tv_usec;
				etv.tv_sec -= stv.tv_sec;
				printf("FE_HAS_LOCK in %u.%06u seconds.\n",
					etv.tv_sec, etv.tv_usec);
				watch = 0;
			}
		}
	}
	close(fd);
	exit(0);
}


--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Devin Heitmueller Jan. 24, 2009, 2:50 a.m. UTC | #7 
On Fri, Jan 23, 2009 at 8:31 PM, Andy Walls <awalls@radix.net> wrote:
> For OTA ATSC 8-VSB with an HVR-1600 MCE:
>
> Without the change:
>
> $ ./tune
> Commanding tune to freq 479028615 ... FE_HAS_LOCK in 1.416984 seconds.
> Commanding tune to freq 551028615 ... FE_HAS_LOCK in 1.389922 seconds.
> Commanding tune to freq 569028615 ... FE_HAS_LOCK in 2.783927 seconds.
> Commanding tune to freq 587028615 ... FE_HAS_LOCK in 1.391952 seconds.
> Commanding tune to freq 593028615 ... NO lock after 2.999655 seconds.
> Commanding tune to freq 599028615 ... FE_HAS_LOCK in 1.568240 seconds.
> Commanding tune to freq 605028615 ... FE_HAS_LOCK in 1.390964 seconds.
> Commanding tune to freq 623028615 ... NO lock after 2.999656 seconds.
> Commanding tune to freq 677028615 ... FE_HAS_LOCK in 2.963289 seconds.
> Commanding tune to freq 695028615 ... NO lock after 2.999618 seconds.
>
> With the change:
>
> $ ./tune
> Commanding tune to freq 479028615 ... FE_HAS_LOCK in 1.323542 seconds.
> Commanding tune to freq 551028615 ... FE_HAS_LOCK in 1.293956 seconds.
> Commanding tune to freq 569028615 ... FE_HAS_LOCK in 1.292931 seconds.
> Commanding tune to freq 587028615 ... FE_HAS_LOCK in 1.292973 seconds.
> Commanding tune to freq 593028615 ... FE_HAS_LOCK in 1.292920 seconds.
> Commanding tune to freq 599028615 ... FE_HAS_LOCK in 1.293977 seconds.
> Commanding tune to freq 605028615 ... FE_HAS_LOCK in 1.292940 seconds.
> Commanding tune to freq 623028615 ... FE_HAS_LOCK in 1.292949 seconds.
> Commanding tune to freq 677028615 ... FE_HAS_LOCK in 1.293948 seconds.
> Commanding tune to freq 695028615 ... NO lock after 2.999659 seconds.
>
>
> No lock was expected for 695 MHz in either case - it was known negative.
>
> Since I was to lazy to get Kaffeine to work properly, I wrote my own
> test app.  It is inline below so you can see how I measured the time.
>
> Regards,
> Andy

Hello Andy,

This is great.  Your data confirms with that device that you're now
getting a lock 100% of the time in a consistent time period.  I
actually got my hands on my own HVR-1600 tonight, and with the
mxl5005s datasheet I got from Maxlinear last week, I will be looking
at lock performance in general for that tuner.

Thanks again for doing this testing.  Mkrufky has indicated he wants
to do some testing this weekend, and assuming that happens I will
submit a PULL request first thing next week.

Devin


--
Devin J. Heitmueller
http://www.devinheitmueller.com
AIM: devinheitmueller
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
A. F. Cano Jan. 24, 2009, 3:07 a.m. UTC | #8 
On Fri, Jan 23, 2009 at 08:31:18PM -0500, Andy Walls wrote:
> ...
> Since I was to lazy to get Kaffeine to work properly, I wrote my own
> test app.  It is inline below so you can see how I measured the time.

As I pointed out in another message recently I've been having problems
tuning my own pvrusb2 device (the OnAir Creator).  I did encounter
problems in kaffeine, so I tried to compile your test app.  A quick
perusal shows that it uses the /dvb/apapter0/* devices so it should
work here, but I can't compile it.  I'm missing some *.h file:

tuner.c: In function ‘main’:
tuner.c:51: error: array type has incomplete element type
tuner.c:52: error: storage size of ‘tasks’ isn’t known
tuner.c:68: error: ‘DTV_DELIVERY_SYSTEM’ undeclared (first use in this function)
tuner.c:68: error: (Each undeclared identifier is reported only once
tuner.c:68: error: for each function it appears in.)
tuner.c:69: error: ‘SYS_ATSC’ undeclared (first use in this function)
tuner.c:71: error: ‘DTV_MODULATION’ undeclared (first use in this function)
tuner.c:76: error: ‘FE_SET_PROPERTY’ undeclared (first use in this function)
tuner.c:96: error: ‘DTV_FREQUENCY’ undeclared (first use in this function)
tuner.c:97: error: ‘DTV_TUNE’ undeclared (first use in this function)

I'm running Debian Lenny.  I did install the libdvb-dev package but that
wasn't it.  There are libdvbpsi[345]-dev packages, but before I go
installing useless packages I thought I'd ask.

A.

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Andy Walls Jan. 24, 2009, 3:29 a.m. UTC | #9 
On Fri, 2009-01-23 at 21:50 -0500, Devin Heitmueller wrote:
> On Fri, Jan 23, 2009 at 8:31 PM, Andy Walls <awalls@radix.net> wrote:
> > For OTA ATSC 8-VSB with an HVR-1600 MCE:
> >
> > Without the change:
> >
> > $ ./tune
> > Commanding tune to freq 479028615 ... FE_HAS_LOCK in 1.416984 seconds.
> > Commanding tune to freq 551028615 ... FE_HAS_LOCK in 1.389922 seconds.
> > Commanding tune to freq 569028615 ... FE_HAS_LOCK in 2.783927 seconds.
> > Commanding tune to freq 587028615 ... FE_HAS_LOCK in 1.391952 seconds.
> > Commanding tune to freq 593028615 ... NO lock after 2.999655 seconds.
> > Commanding tune to freq 599028615 ... FE_HAS_LOCK in 1.568240 seconds.
> > Commanding tune to freq 605028615 ... FE_HAS_LOCK in 1.390964 seconds.
> > Commanding tune to freq 623028615 ... NO lock after 2.999656 seconds.
> > Commanding tune to freq 677028615 ... FE_HAS_LOCK in 2.963289 seconds.
> > Commanding tune to freq 695028615 ... NO lock after 2.999618 seconds.
> >
> > With the change:
> >
> > $ ./tune
> > Commanding tune to freq 479028615 ... FE_HAS_LOCK in 1.323542 seconds.
> > Commanding tune to freq 551028615 ... FE_HAS_LOCK in 1.293956 seconds.
> > Commanding tune to freq 569028615 ... FE_HAS_LOCK in 1.292931 seconds.
> > Commanding tune to freq 587028615 ... FE_HAS_LOCK in 1.292973 seconds.
> > Commanding tune to freq 593028615 ... FE_HAS_LOCK in 1.292920 seconds.
> > Commanding tune to freq 599028615 ... FE_HAS_LOCK in 1.293977 seconds.
> > Commanding tune to freq 605028615 ... FE_HAS_LOCK in 1.292940 seconds.
> > Commanding tune to freq 623028615 ... FE_HAS_LOCK in 1.292949 seconds.
> > Commanding tune to freq 677028615 ... FE_HAS_LOCK in 1.293948 seconds.
> > Commanding tune to freq 695028615 ... NO lock after 2.999659 seconds.
> >
> >
> > No lock was expected for 695 MHz in either case - it was known negative.
> >
> > Since I was to lazy to get Kaffeine to work properly, I wrote my own
> > test app.  It is inline below so you can see how I measured the time.
> >
> > Regards,
> > Andy
> 
> Hello Andy,
> 
> This is great.  Your data confirms with that device that you're now
> getting a lock 100% of the time in a consistent time period.

Yup.

Strangely enough, the measurments for both cases yield very repeatable
results.  The "bad" numbers always come out in about the same relative
times.  I suppose that might change, if I changed the order of which
frequencies were tuned.


>   I
> actually got my hands on my own HVR-1600 tonight, and with the
> mxl5005s datasheet I got from Maxlinear last week, I will be looking
> at lock performance in general for that tuner.

I think you might need to pry the can open for that one.  Looking at the
linux driver code, I get the feeling that external components can be set
up differently for various tracking filter configurations.  Of course,
Steve Toth added the Hauppauge variant tracking filter settings for the
HVR-1600.


> Thanks again for doing this testing.

No problem. 

Let me know if there's something else you need tested.  Now that I've
bothered to write a DVB API v5 test app, I've actually started to read
the DVB API v3 document and the API v5 code.  I understand some of the
linux DVB internal architecture concepts now.  (My day job has also had
me reading the ETSI DVB-S2 standards documentation this past week
anyway.)


>   Mkrufky has indicated he wants
> to do some testing this weekend, and assuming that happens I will
> submit a PULL request first thing next week.
> 
> Devin

Regards,
Andy

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Andy Walls Jan. 24, 2009, 3:39 a.m. UTC | #10 
On Fri, 2009-01-23 at 22:07 -0500, A. F. Cano wrote:
> On Fri, Jan 23, 2009 at 08:31:18PM -0500, Andy Walls wrote:
> > ...
> > Since I was to lazy to get Kaffeine to work properly, I wrote my own
> > test app.  It is inline below so you can see how I measured the time.
> 
> As I pointed out in another message recently I've been having problems
> tuning my own pvrusb2 device (the OnAir Creator).  I did encounter
> problems in kaffeine, so I tried to compile your test app.  A quick
> perusal shows that it uses the /dvb/apapter0/* devices so it should
> work here, but I can't compile it.  I'm missing some *.h file:
> 
> tuner.c: In function ‘main’:
> tuner.c:51: error: array type has incomplete element type
> tuner.c:52: error: storage size of ‘tasks’ isn’t known
> tuner.c:68: error: ‘DTV_DELIVERY_SYSTEM’ undeclared (first use in this function)
> tuner.c:68: error: (Each undeclared identifier is reported only once
> tuner.c:68: error: for each function it appears in.)
> tuner.c:69: error: ‘SYS_ATSC’ undeclared (first use in this function)
> tuner.c:71: error: ‘DTV_MODULATION’ undeclared (first use in this function)
> tuner.c:76: error: ‘FE_SET_PROPERTY’ undeclared (first use in this function)
> tuner.c:96: error: ‘DTV_FREQUENCY’ undeclared (first use in this function)
> tuner.c:97: error: ‘DTV_TUNE’ undeclared (first use in this function)
> 
> I'm running Debian Lenny.  I did install the libdvb-dev package but that
> wasn't it.  There are libdvbpsi[345]-dev packages, but before I go
> installing useless packages I thought I'd ask.

The latest dvb/frontend.h header file that includes the DVB API v5
ioctl()s aren't include in most distributions yet.


Here's how I compiled it, pointing to an include directory inside my
development repository:

$ gcc -Wall tune.c -o tune -I/home/andy/cx18dev/v4l-dvb/linux/include

Be aware that my little app sets up ATSC 8-VSB modulation which is the
driver default for my card anyway.  Even if I left some settings out, I
knew things would "just work".

Also note that I have a hard coded frequency/channel table in the
program useful for the terrestrial broadcasters near me, not necessarily
near you.

Regards,
Andy


--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

s5h1409: Perform s5h1409 soft reset after tuning

From: Devin Heitmueller <dheitmueller@linuxtv.org>

Just like with the s5h1411, the s5h1409 needs a soft-reset in order for it
to know that the tuner has been told to change frequencies.  This change
changes the behavior from "random tuning times between 500ms to complete 
tuning lock failures" to "tuning lock consistently within 700ms".

Thanks to Robert Krakora <rob.krakora@messagenetsystems.com> for doing 
initial testing of the patch on the KWorld 330U.

Signed-off-by: Devin Heitmueller <dheitmueller@linuxtv.org>
diff -r 4a06b5c3344f linux/drivers/media/dvb/frontends/s5h1409.c
--- a/linux/drivers/media/dvb/frontends/s5h1409.c	Mon Dec 29 22:17:09 2008 -0500
+++ b/linux/drivers/media/dvb/frontends/s5h1409.c	Mon Jan 19 19:50:29 2009 -0500
@@ -545,9 +545,6 @@ 
 
 	s5h1409_enable_modulation(fe, p->u.vsb.modulation);
 
-	/* Allow the demod to settle */
-	msleep(100);
-
 	if (fe->ops.tuner_ops.set_params) {
 		if (fe->ops.i2c_gate_ctrl)
 			fe->ops.i2c_gate_ctrl(fe, 1);
@@ -561,6 +558,10 @@ 
 		s5h1409_set_qam_amhum_mode(fe);
 		s5h1409_set_qam_interleave_mode(fe);
 	}
+
+	/* Issue a reset to the demod so it knows to resync against the
+	   newly tuned frequency */
+	s5h1409_softreset(fe);
 
 	return 0;
 }