On Tuesday 05 July 2005 09:37 am, Michael Krufky wrote:
> Mac Michaels wrote:
> >I do not approve this patch. Mac Michaels
> >
> >--- linux-2.6.13/drivers/media/video/cx88/cx88-mpeg.c
> > 2005-07-05 00:34:56.000000000 +0000 +++
> > linux/drivers/media/video/cx88/cx88-mpeg.c 2005-07-05
> > 00:47:22.000000000 +0000 @@ -73,11 +73,11 @@
> > cx_write(TS_GEN_CNTRL, 0x0040 |
> > dev->ts_gen_cntrl); udelay(100);
> > cx_write(MO_PINMUX_IO, 0x00);
> >- if (core->board ==
> > CX88_BOARD_DVICO_FUSIONHDTV_3_GOLD_Q) { [1]-
> > cx_write(TS_HW_SOP_CNTRL,0x47<<16 | 188<<4 |
> > 0x00); [2]+
> > cx_write(TS_HW_SOP_CNTRL,47<<16|188<<4|0x00); +
> > if ((core->board ==
> > CX88_BOARD_DVICO_FUSIONHDTV_3_GOLD_Q) || +
> > (core->board ==
> > CX88_BOARD_DVICO_FUSIONHDTV_3_GOLD_T)) {
> > cx_write(TS_SOP_STAT, 0<<16 | 0<<14 | 1<<13 | 0<<12); }
> > else {
> >[3]-
> > cx_write(TS_HW_SOP_CNTRL,47<<16|188<<4|0x00);
> > cx_write(TS_SOP_STAT,0x00); }
> > cx_write(TS_GEN_CNTRL,
> > dev->ts_gen_cntrl);
> >
> >I added numbers in braces for reference in the following
> > discussion. It appears that Michael thought lines [1]
> > and [3] were identical. He simplified the code by
> > combining them in one place at line [2]. Look carefully
> > at the hexadecimal 0x47 constant in the middle of line
> > [1] It the value required for FusionHDTV 3 Gold cards.
> > Lines [2] and [3] contain the decimal 47 constant. This
> > part of the patch should not be applied.
> >
> >As an aside, I wonder if all cards should use the 0x47
> > constant as it is listed as the default start byte
> > value for ATSC. It is not actually transmitted but is
> > generated by the LGDT3302 chip. I guess other chips
> > could use a different value if they want to. That is
> > why I chose not to change the code for cards other than
> > FusionHDTV 3 Gold.
>
> Mac-
>
> The change above is already in video4linux cvs. Please
> test from cvs to confirm that it should be reverted. I
> was talking to Mauro about this and we were both
> wondering why you had changed from decimal 47 to Hex 0x47
> , where it seems that decimal 47 should be correct based
> on all other calls... I have tried both values on my
> board and noticed no difference. If this actually causes
> a problem for you, I will revert this change. I am not
> sending this to Andrew Morton or LKML until this is all
> resolved :-)
>
> Let me know.
This is going to take a bit of explanation. Short answer - I am correct and it seems to work anyway.
There are two control registers that control the detection of start of packet (SOP) - TS_HW_SOP_CNTRL and
TS_SOP_STAT. Both of these registers are documented in the CX23880 data sheet.
cx_write(TS_HW_SOP_CNTRL,47<<16|188<<4|0x00);
is interpreted as Packet start byte value is 47, Packet length is 188, 0 start bytes must be received before declaring the packet stream in sync. Logic indicates that if 0 start bytes must be received then the value of the start byte
does not matter. It is in sync all the time. This is a bug and should be fixed.
Changing the line to:
cx_write(TS_HW_SOP_CNTRL,47<<16|188<<4|0x01);
will appear to work since somewhere in a small number of packets it is likely that the decimal 47 will appear. It does not need to be the first byte. I only needs to appear once to have the stream declared in sync. Actual start of packet is done by another hardware signal.
How does this get in sync? That is where TS_SOP_STAT is used.
cx_write(TS_SOP_STAT, 0<<16 | 0<<14 | 1<<13 | 0<<12);
Bit 16 selects active high polarity. Bits 15,14 select SOP detects rising edge of TSSCP input, BIT 13 selects width of SOP as byte in serial mode (FusionHDTV 3 Gold uses serial mode), Bit 12 was used when I was experimenting and may be removed as the default is 0. This code does not need to be changed in the driver.
An interesting option for bits 15,14 is 3. This uses the Packet start byte value set in the TS_HW_SOP_CNTRL register to locate the start of a packet. This is intended for hardware that does not have connection to TSSOP or TSVALERR suitable for detecting the start of a packet. It is interesting for testing the value of the SOP byte value.
Here are two variations I tried:
cx_write(TS_HW_SOP_CNTRL,47<<16|188<<4|0x01);
cx_write(TS_SOP_STAT, 0<<16 | 3<<14 | 1<<13 | 0<<12);
Result: no packets detected.
cx_write(TS_HW_SOP_CNTRL,0x47<<16|188<<4|0x01);
cx_write(TS_SOP_STAT, 0<<16 | 3<<14 | 1<<13 | 0<<12);
Result: packets detected and normal operation of mythtv.
If you can verify that the other frontends use the normal default value for the packet start byte, you could do this:
- cx_write(TS_HW_SOP_CNTRL,47<<16|188<<4|0x00);
+ cx_write(TS_HW_SOP_CNTRL,0x47<<16|188<<4|0x01);
Here is my paranoid recommendation for fixing the code in CVS:
Signed off by Mac Michaels <wmichaels1@xxxxxxxxxxxxx>
Index: cx88-mpeg.c
===================================================================
RCS file: /cvs/video4linux/video4linux/cx88-mpeg.c,v
retrieving revision 1.29
diff -u -b -B -u -r1.29 cx88-mpeg.c
--- cx88-mpeg.c 4 Jul 2005 19:35:05 -0000 1.29
+++ cx88-mpeg.c 5 Jul 2005 16:58:36 -0000
@@ -73,11 +73,12 @@
cx_write(TS_GEN_CNTRL, 0x0040 | dev->ts_gen_cntrl);
udelay(100);
cx_write(MO_PINMUX_IO, 0x00);
- cx_write(TS_HW_SOP_CNTRL,47<<16|188<<4|0x00);
if ((core->board == CX88_BOARD_DVICO_FUSIONHDTV_3_GOLD_Q) ||
(core->board == CX88_BOARD_DVICO_FUSIONHDTV_3_GOLD_T)) {
+ cx_write(TS_HW_SOP_CNTRL,0x47<<16|188<<4|0x01);
cx_write(TS_SOP_STAT, 0<<16 | 0<<14 | 1<<13 | 0<<12);
} else {
+ cx_write(TS_HW_SOP_CNTRL,47<<16|188<<4|0x00);
cx_write(TS_SOP_STAT,0x00);
}
cx_write(TS_GEN_CNTRL, dev->ts_gen_cntrl);
