[PATCH] adv717x module code big reorganization - part 2 -- please test and/or comment

[Nicolas Boullis]

Hi,

I've done more work for the adv717x code reorganization. There should
still be absolutely no behavioral difference, but the code is getting
less em8300-dependant.

Please try the attached patch and report. Note that this patch is on top
of current CVS (that is over the first part of the adv717x code
reorganization).


Cheers,

Nicolas


-------------------------------------------------------------------------
SF.Net email is sponsored by: The Future of Linux Business White Paper
from Novell.  From the desktop to the data center, Linux is going
mainstream.  Let it simplify your IT future.
http://altfarm.mediaplex.com/ad/ck/8857-50307-18918-4
_______________________________________________
Dxr3-devel mailing list
Dxr3-devel@...
https://lists.sourceforge.net/lists/listinfo/dxr3-devel

[Ville Skyttä]

On Wednesday 05 December 2007, Nicolas Boullis wrote:
> Hi,
>
> I've done more work for the adv717x code reorganization. There should
> still be absolutely no behavioral difference, but the code is getting
> less em8300-dependant.
>
> Please try the attached patch and report. Note that this patch is on top
> of current CVS (that is over the first part of the adv717x code
> reorganization).

After applying this patch, I get everything in black and white.  Reverting to
current CVS brings the colors back.

-------------------------------------------------------------------------
SF.Net email is sponsored by:
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://sourceforge.net/services/buy/index.php
_______________________________________________
Dxr3-devel mailing list
Dxr3-devel@...
https://lists.sourceforge.net/lists/listinfo/dxr3-devel

[Nicolas Boullis]

Hi,

On Fri, Dec 07, 2007 at 11:02:31PM +0200, Ville Skyttä wrote:
>
> After applying this patch, I get everything in black and white.  Reverting to
> current CVS brings the colors back.

Di you actualy rebuild both the em8300 and the adv717x modules, and load
both new versions?
With this patch, the adv717x module relies on the em8300 one for proper
configuration.


Cheers,

Nicolas

-------------------------------------------------------------------------
SF.Net email is sponsored by:
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://sourceforge.net/services/buy/index.php
_______________________________________________
Dxr3-devel mailing list
Dxr3-devel@...
https://lists.sourceforge.net/lists/listinfo/dxr3-devel

[Ville Skyttä]

On Saturday 08 December 2007, Nicolas Boullis wrote:
> Hi,
>
> On Fri, Dec 07, 2007 at 11:02:31PM +0200, Ville Skyttä wrote:
> > After applying this patch, I get everything in black and white.
> > Reverting to current CVS brings the colors back.
>
> Di you actualy rebuild both the em8300 and the adv717x modules,

Yes.  I always build even test versions as rpm packages from scratch in a
clean buildroot, so stale builds aren't really even possible.

> and load both new versions?

Yes, tried several times, even rebooted to be sure.  Tested with VDR in case
it could matter.

> With this patch, the adv717x module relies on the em8300 one for proper
> configuration.

Is there something different I should be doing eg. in modprobe.conf compared
to pre-patch versions?  What I currently have there is (pardon the line
wrapping):

remove em8300 /sbin/modprobe -r --ignore-remove em8300 && /sbin/modprobe -r
adv717x
install em8300 /sbin/modprobe adv717x && /sbin/modprobe --ignore-install
em8300
options em8300 dicom_other_pal=0 dicom_control=0 audio_driver=oss
options adv717x pixelport_other_pal=0 output_mode=yuv

-------------------------------------------------------------------------
SF.Net email is sponsored by:
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://sourceforge.net/services/buy/index.php
_______________________________________________
Dxr3-devel mailing list
Dxr3-devel@...
https://lists.sourceforge.net/lists/listinfo/dxr3-devel

[Nicolas Boullis]

Hi,

On Sat, Dec 08, 2007 at 10:44:57AM +0200, Ville Skyttä wrote:

> On Saturday 08 December 2007, Nicolas Boullis wrote:
> > Hi,
> >
> > On Fri, Dec 07, 2007 at 11:02:31PM +0200, Ville Skyttä wrote:
> > > After applying this patch, I get everything in black and white.
> > > Reverting to current CVS brings the colors back.
> >
> > Di you actualy rebuild both the em8300 and the adv717x modules,
>
> Yes.  I always build even test versions as rpm packages from scratch in a
> clean buildroot, so stale builds aren't really even possible.
>
> > and load both new versions?
>
> Yes, tried several times, even rebooted to be sure.  Tested with VDR in case
> it could matter.
>
> > With this patch, the adv717x module relies on the em8300 one for proper
> > configuration.
>
> Is there something different I should be doing eg. in modprobe.conf compared
> to pre-patch versions?  What I currently have there is (pardon the line
> wrapping):
>
> remove em8300 /sbin/modprobe -r --ignore-remove em8300 && /sbin/modprobe -r
> adv717x
> install em8300 /sbin/modprobe adv717x && /sbin/modprobe --ignore-install
> em8300
> options em8300 dicom_other_pal=0 dicom_control=0 audio_driver=oss
> options adv717x pixelport_other_pal=0 output_mode=yuv

OK, everything looks fine on your side. I hoped you had made a mistake,
but I guess the eal problem lies in my patch...

Can you try the attached patch instead (the only difference is some
printk) and report with the kernel log for the modules' insertion?


Cheers,

Nicolas


-------------------------------------------------------------------------
SF.Net email is sponsored by:
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://sourceforge.net/services/buy/index.php
_______________________________________________
Dxr3-devel mailing list
Dxr3-devel@...
https://lists.sourceforge.net/lists/listinfo/dxr3-devel

[Ville Skyttä]

On Sunday 09 December 2007, Nicolas Boullis wrote:

> Can you try the attached patch instead (the only difference is some
> printk) and report with the kernel log for the modules' insertion?

Sure.

kernel: ACPI: PCI Interrupt 0000:02:04.0[A] -> GSI 16 (level, low) -> IRQ 20
kernel: em8300: EM8300 8300 (rev 2) bus: 2, devfn: 32, irq: 20, memory:
0xe7000000.
kernel: em8300: mapped-memory at 0xe0c80000
em8300: using MTRR
kernel: em8300_main.o: Chip revision: 2
kernel: adv717x.o: ADV7170 chip detected
kernel: adv717x.c: ENCODER_CMD_GETCONFIG => 1 0 1 1 0 6
kernel: adv717x.c: ENCODER_CMD_SETPARAM(1, -1, 0) => 0
kernel: adv717x.c: ENCODER_CMD_SETPARAM(2, -1, 1) => 0
kernel: adv717x.c: ENCODER_CMD_SETPARAM(3, 2, 1) => 0
kernel: adv717x.c: ENCODER_CMD_SETPARAM(3, 104, 1) => 0
kernel: adv717x.c: ENCODER_CMD_SETPARAM(4, 2, 1) => 0
kernel: adv717x.c: ENCODER_CMD_SETPARAM(4, 104, 1) => 0

BTW, the "EM8300 8300" looks a bit odd, is it supposed to be like that?

-------------------------------------------------------------------------
SF.Net email is sponsored by:
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://sourceforge.net/services/buy/index.php
_______________________________________________
Dxr3-devel mailing list
Dxr3-devel@...
https://lists.sourceforge.net/lists/listinfo/dxr3-devel

[Sebastian Frei-2]

Am Samstag, 8. Dezember 2007 23:43:33 schrieb Nicolas Boullis:

> Can you try the attached patch instead (the only difference is some
> printk) and report with the kernel log for the modules' insertion?
>
>
> Cheers,
>
> Nicolas

The second patch works without any problems too.
I also have colours with VDR.
Kernel 2.4.24-rc4

S.

CI: Enabling device 0000:00:0d.0 (0004 -> 0006)
ACPI: PCI Interrupt Link [LNKA] enabled at IRQ 11
PCI: setting IRQ 11 as level-triggered
ACPI: PCI Interrupt 0000:00:0d.0[A] -> Link [LNKA] -> GSI 11 (level, low) ->
IRQ 11
em8300: EM8300 8300 (rev 1) bus: 0, devfn: 104, irq: 11, memory: 0xf4000000.
em8300: mapped-memory at 0xc4880000
em8300: using MTRR
em8300_main.o: Chip revision: 2
adv717x.o: ADV7170 chip detected
adv717x.c: ENCODER_CMD_GETCONFIG => 0 0 1 1 0 0
adv717x.c: ENCODER_CMD_SETPARAM(1, -1, 0) => 0
adv717x.c: ENCODER_CMD_SETPARAM(2, -1, 0) => 0
adv717x.c: ENCODER_CMD_SETPARAM(3, 2, 0) => 0
adv717x.c: ENCODER_CMD_SETPARAM(3, 104, 0) => 0
adv717x.c: ENCODER_CMD_SETPARAM(4, 2, 1) => 0
adv717x.c: ENCODER_CMD_SETPARAM(4, 104, 1) => 0
em8300_audio.o: Analog audio enabled
em8300: Microcode version 0x29 loaded
em8300_audio.o: Analog audio enabled
em8300: Microcode version 0x29 loaded
em8300_audio.o: Analog audio enabled

-------------------------------------------------------------------------
SF.Net email is sponsored by:
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://sourceforge.net/services/buy/index.php
_______________________________________________
Dxr3-devel mailing list
Dxr3-devel@...
https://lists.sourceforge.net/lists/listinfo/dxr3-devel

[Nicolas Boullis]

On Sun, Dec 09, 2007 at 11:58:50AM +0200, Ville Skyttä wrote:

>
> kernel: ACPI: PCI Interrupt 0000:02:04.0[A] -> GSI 16 (level, low) -> IRQ 20
> kernel: em8300: EM8300 8300 (rev 2) bus: 2, devfn: 32, irq: 20, memory:
> 0xe7000000.
> kernel: em8300: mapped-memory at 0xe0c80000
> em8300: using MTRR
> kernel: em8300_main.o: Chip revision: 2
> kernel: adv717x.o: ADV7170 chip detected
> kernel: adv717x.c: ENCODER_CMD_GETCONFIG => 1 0 1 1 0 6
> kernel: adv717x.c: ENCODER_CMD_SETPARAM(1, -1, 0) => 0
> kernel: adv717x.c: ENCODER_CMD_SETPARAM(2, -1, 1) => 0
> kernel: adv717x.c: ENCODER_CMD_SETPARAM(3, 2, 1) => 0
> kernel: adv717x.c: ENCODER_CMD_SETPARAM(3, 104, 1) => 0
> kernel: adv717x.c: ENCODER_CMD_SETPARAM(4, 2, 1) => 0
> kernel: adv717x.c: ENCODER_CMD_SETPARAM(4, 104, 1) => 0

OK, I think I found my stupid mistake, and if I'm right it was
outputting svideo instead of yuv...

Can you please check this new attached patch...


> BTW, the "EM8300 8300" looks a bit odd, is it supposed to be like that?

Hmmm, that's a good question. I guess I should give it a look, but I
don't think it matters much.


Cheers,

Nicolas

-------------------------------------------------------------------------
SF.Net email is sponsored by:
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://sourceforge.net/services/buy/index.php
_______________________________________________
Dxr3-devel mailing list
Dxr3-devel@...
https://lists.sourceforge.net/lists/listinfo/dxr3-devel

[Nicolas Boullis]

On Sun, Dec 09, 2007 at 06:30:09PM +0100, Nicolas Boullis wrote:
>
> Can you please check this new attached patch...

I meant this one. ;-)


Nicolas


-------------------------------------------------------------------------
SF.Net email is sponsored by:
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://sourceforge.net/services/buy/index.php
_______________________________________________
Dxr3-devel mailing list
Dxr3-devel@...
https://lists.sourceforge.net/lists/listinfo/dxr3-devel

[Ville Skyttä]

On Sunday 09 December 2007, Nicolas Boullis wrote:

> On Sun, Dec 09, 2007 at 11:58:50AM +0200, Ville Skyttä wrote:
> > kernel: ACPI: PCI Interrupt 0000:02:04.0[A] -> GSI 16 (level, low) -> IRQ
> > 20 kernel: em8300: EM8300 8300 (rev 2) bus: 2, devfn: 32, irq: 20,
> > memory: 0xe7000000.
> > kernel: em8300: mapped-memory at 0xe0c80000
> > em8300: using MTRR
> > kernel: em8300_main.o: Chip revision: 2
> > kernel: adv717x.o: ADV7170 chip detected
> > kernel: adv717x.c: ENCODER_CMD_GETCONFIG => 1 0 1 1 0 6
> > kernel: adv717x.c: ENCODER_CMD_SETPARAM(1, -1, 0) => 0
> > kernel: adv717x.c: ENCODER_CMD_SETPARAM(2, -1, 1) => 0
> > kernel: adv717x.c: ENCODER_CMD_SETPARAM(3, 2, 1) => 0
> > kernel: adv717x.c: ENCODER_CMD_SETPARAM(3, 104, 1) => 0
> > kernel: adv717x.c: ENCODER_CMD_SETPARAM(4, 2, 1) => 0
> > kernel: adv717x.c: ENCODER_CMD_SETPARAM(4, 104, 1) => 0
>
> OK, I think I found my stupid mistake, and if I'm right it was
> outputting svideo instead of yuv...

Could be, didn't check though.

> Can you please check this new attached patch...

This one appears to work fine.  Thanks.

adv717x.c: ENCODER_CMD_GETCONFIG => 1 0 1 1 0 6
adv717x.c: ENCODER_CMD_SETPARAM(1, -1, 0) => 0
adv717x.c: ENCODER_CMD_SETPARAM(2, -1, 6) => 0
adv717x.c: ENCODER_CMD_SETPARAM(3, 2, 1) => 0
adv717x.c: ENCODER_CMD_SETPARAM(3, 104, 1) => 0
adv717x.c: ENCODER_CMD_SETPARAM(4, 2, 1) => 0
adv717x.c: ENCODER_CMD_SETPARAM(4, 104, 1) => 0

-------------------------------------------------------------------------
SF.Net email is sponsored by:
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://sourceforge.net/services/buy/index.php
_______________________________________________
Dxr3-devel mailing list
Dxr3-devel@...
https://lists.sourceforge.net/lists/listinfo/dxr3-devel

[Nicolas Boullis]

Hi,

On Sun, Dec 09, 2007 at 11:06:28PM +0200, Ville Skyttä wrote:
>
> This one appears to work fine.  Thanks.

OK, I just committed this patch (without the printks I added to debug
your problem). Thanks to you and Sebastian Frei for testing my patch.


Nicolas

-------------------------------------------------------------------------
SF.Net email is sponsored by:
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://sourceforge.net/services/buy/index.php
_______________________________________________
Dxr3-devel mailing list
Dxr3-devel@...
https://lists.sourceforge.net/lists/listinfo/dxr3-devel