Bug 8811

Summary: TV-in for All in Wonder 128
Product: xorg Reporter: Michael Fritscher <michael>
Component: Driver/rage128Assignee: Alex Deucher <alexdeucher>
Status: RESOLVED WONTFIX QA Contact: Xorg Project Team <xorg-team>
Severity: enhancement    
Priority: high CC: alexdeucher, eich, mat, sbrabec, sndirsch, utx
Version: gitKeywords: patch
Hardware: x86 (IA32)   
OS: Linux (All)   
URL: http://mifritscher.de/ati2.patch.gz
Whiteboard:
i915 platform: i915 features:
Attachments:
Description Flags
xf86-video-ati-6.6.3-r128-xvideo.patch
none
xf86-video-ati-6.6.3-compile-cfb.patch
none
xf86-video-ati-6.6.3-if0.patch
none
xf86-video-ati-6.7.197-r128-xvideo.patch
none
xf86-video-ati-theatre.patch
none
xf86-video-ati-theatre-pciaccess.patch none

Description Michael Fritscher 2006-10-28 17:48:25 UTC
I merged the ati and r128 parts from the ati.2-driver from gatos with current
git from x.org.
It is fully functional you can show tv with avview.
In addition with the km-driver capture is possible, too.

With this patch the driver needs the i2c and detect_theatre module.
Comment 1 Alex Deucher 2006-11-01 11:47:03 UTC
post your patch so it can be reviewed for inclusion.  Make sure you are not
merging any GPLed code.
Comment 2 Daniel Stone 2006-11-01 12:37:22 UTC
it's already there, in the url field

the usual coding style issues, and apparently dri + xv = crash.
Comment 3 Alex Deucher 2006-11-01 12:53:01 UTC
(In reply to comment #2)
> it's already there, in the url field
> 

Ah missed that.

> the usual coding style issues, and apparently dri + xv = crash.

that's probably the same bug that was in there when Vladimir released a similar
patch two years ago:
http://lists.freedesktop.org/pipermail/xorg/2004-December/005371.html
http://lists.freedesktop.org/pipermail/xorg/2005-January/005609.html
Probably a CCE wait-for-idle before the CCE is initialized.

Comment 4 Michael Fritscher 2006-11-19 06:13:17 UTC
How can I fix that?
Sorry, I'm a beginner hacking the x.org-code :-)
Comment 5 Alex Deucher 2006-11-20 10:57:53 UTC
(In reply to comment #4)
> How can I fix that?
> Sorry, I'm a beginner hacking the x.org-code :-)

Go through the patched r128 driver code and look for macros that try and idle
the CCE before it is initialized.  I'd start with r128preinit() in r128_driver.c
and move out from there.  You can take a look at the radeon code for reference.
Comment 6 Daniel Stone 2007-02-27 01:34:13 UTC
Sorry about the phenomenal bug spam, guys.  Adding xorg-team@ to the QA contact so bugs don't get lost in future.
Comment 7 Stanislav Brabec 2007-03-04 08:11:17 UTC
Created attachment 8969 [details] [review]
xf86-video-ati-6.6.3-r128-xvideo.patch

Here is my patch based on the same code.
Not knowing about this bug, I have created another patch from scratch.

I spent several days in diff clean-up and removed all code, where GATOS chunk contained older code. My code does not change any file without r128_ in name.

It works on my AIW R128 32MB. Tested on TV-input.

It is a patch against xf86-video-ati-6.6.3 from openSUSE 10.2.

Known issue:
- No support for stereo decoder
Comment 8 Stanislav Brabec 2007-03-04 08:36:06 UTC
My patch works with DRI.

Support for Rage Theatre 200 was not added to my patch. I doubt there is any R128 + RT200 card.

Another issue I found. I am not sure that I will be able to fix it:
- Use of XRandR or screen mode change freezes overlay video.

I will verify these issues known from older versions of R128 drivers (not GATOS related):
- Use of GL or overlay video sometimes corrupts hw cursor colormap.
- Ugly graphic effects while starting the server (cosmetical regression sometimes between xorg 6.8.1 and 6.9.0).
- Display corruption when overlay video is completely off-screen (not seen a long time, probably does not occur any more).
- Compare Radeon and R128 XV_* controls and scales and sync them.
- Improve video size XV_* controls for capture to allow arbitrary sizes.
- Verify, whether it can still be compiled without XVideo support.
Comment 9 Matthias Hopf 2007-06-08 05:52:46 UTC
Some of the known issues are serious:

- Use of XRandR or screen mode change freezes overlay video.
- Use of GL or overlay video sometimes corrupts hw cursor colormap.

As long as these are not fixed I hesitate from applying. At least RandR shouldn't freeze anything. Does RandR *only* freeze the overlay video, and can it be restarted? In that case we could think about it.


Stanislavs comment:

>- Use of XRandR or screen mode change freezes overlay video.
Problem of my patch. After xrandr call Xvideo window displays static
image. It is enough to restart application, which was using Xvideo; no
crash or server freeze.
Problem is not critical. I can try to fix it in future by disabling
Xvideo before screen mode change.

>- Use of GL or overlay video sometimes corrupts hw cursor colormap.
Perfectly reproducible without my patch.

[...] same with the other issues.


So no regressions, except for the RandR problem. I would like to have that fixed (so it at least works as before). Also I have a couple of other points with that patch.

Given that R128 isn't that widely used any more, and we probably won't get better integration of the GATOS features in the near future, I would commit the changes after the issues have been addressed - if nobody objects.

Michael, any chance you could try Stanislav's patch instead of yours and try whether it works for you as well? It's much cleaner, and will probably provide less headaches after being committed...
Comment 10 Michael Fritscher 2007-06-08 11:29:37 UTC
Sorry for the delay, I have a lot of abi-stress atm. Next weak it will be done :-)
Whiletime, X.org was upgraded to 7.2 (ubuntu), and if I saw right this patch was for X.org 6.8?

I'll try it next week.

Many thanks,
Michael Fritscher
Comment 11 Stanislav Brabec 2007-06-28 14:04:16 UTC
Created attachment 10492 [details] [review]
xf86-video-ati-6.6.3-compile-cfb.patch

This incremental patch makes cfb compiling again. But it does not mean, that it will work. In 8bpp mode, all applications crash, in 16bpp and 24bpp mode nothing crashes, but some elements are imporperly half scaled and the result is unusable.
I guess everybody can live without cfb code, because it is disabled for years.
Comment 12 Stanislav Brabec 2007-06-28 14:09:19 UTC
Created attachment 10493 [details] [review]
xf86-video-ati-6.6.3-if0.patch

Incremental patch to explain code I was pointed to by Matthias Hopf:

I'm a bit startled by this part of the patch:

@@ -4454,11 +4647,14 @@
     if (info->DGAModes)          xfree(info->DGAModes);
     info->DGAModes               = NULL;
 
+#if 0
     if (info->adaptor) {
+       R128ShutdownVideo(pScrn, info->adaptor->pPortPrivates[0].ptr);
         xfree(info->adaptor->pPortPrivates[0].ptr);
        xf86XVFreeVideoAdaptorRec(info->adaptor);
        info->adaptor = NULL;
     }
+#endif
 
     pScrn->vtSema = FALSE;

I don't remove this code, because I did not traced, what is the call sequence during card shutdown and whether this code is bad or only needs fix.

The crash is:

0: X(xf86SigHandler+0x81) [0x80cbf81]
1: [0xb7f0d420]
2: /usr/lib/xorg/modules/multimedia//theatre_drv.so [0xb7adaf69]
3: /usr/lib/xorg/modules/multimedia//theatre_drv.so [0xb7adb011]
4: /usr/lib/xorg/modules/multimedia//theatre_drv.so(ShutdownTheatre+0x1b) [0xb7adc13b]
5: /usr/lib/xorg/modules/drivers//r128_drv.so(R128ShutdownVideo+0x74) [0xb7b28d59]
6: /usr/lib/xorg/modules/drivers//r128_drv.so [0xb7b28237]
7: X [0x81549d7]
8: X [0x80c6bfc]
9: X [0x80c0e59]
10: X [0x812d698]
11: X [0x8151865]
12: X [0x80f0a25]
13: X(main+0x50b) [0x806e8cb]
14: /lib/libc.so.6(__libc_start_main+0xdc) [0xb7c8cf9c]
15: X(FontFileCompleteXLFD+0x1e1) [0x806db71]
Comment 13 Matthias Hopf 2007-07-04 02:40:50 UTC
Ok, so you disabled the code because it crashed on your hardware? Fair enough :)

I guess this is just the usual bitrot.
Comment 14 Stanislav Brabec 2007-07-04 04:01:54 UTC
Both codes in last two patches were already disabled for years. Searching in GATOS and linuxvideo CVS:


For shutting down the card:

Adding R128ShutDownVideo:
revision 1.8
date: 2001-06-01 13:50:25 +0200;  author: volodya;  state: Exp;  lines: +1 -0;
Further down Marcs and Kevin suggestions list.

Commenting out:
revision 1.9
date: 2001-06-02 00:40:42 +0200;  author: volodya;  state: Exp;  lines: +10 -0;
Further cleanups. Radeon (verified) and R128(untested) behave ok during VT switches.

I only added comment, why it is disabled. I have not enough insight to guess, whether it is disabled because it is obsolete or broken.


For USE_FB:

As far as CVS history can show, CFB code was disabled (with comment "not until overlays and 24->32 code added", later only "not until overlays"). The code was completely broken again by upgrade to XFree-4.0.0.

I only made the cfb code to compile again (if it is enabled) and added comment, that it is broken.
Comment 15 Alex Deucher 2007-07-04 06:01:45 UTC
Please don't bring back cfb.  Is there any reason you need it?
Comment 16 Stanislav Brabec 2007-07-04 06:39:54 UTC
I don't bring it back. It is still disabled and broken. It was only a research why these parts of code are disabled and what happens if I try to enable disabled code.

After finishing the research I can say:

- The default setting coming from GATOS works

- The setting with XVideo disabled works

- Enabling other disabled parts of code may break the driver
Comment 17 Matthias Hopf 2007-07-04 10:11:57 UTC
Apart from the RandR issue I have no problems applying this patch any more. I just would like to hear from Michael first, whether he has additional issues.

It would be great, of course, if the RandR problem could be fixed as well...
Comment 18 Michael Fritscher 2007-07-04 16:52:51 UTC
Do you have a binary module? I'm afraid my x.org devel-tree is borged atm...
Else I must clean up this mess first...
I'm using ubuntu 7.04 with its xorg 7.2 now.
Comment 19 Alex Deucher 2007-07-05 17:12:31 UTC
When you switch modes the overlay gets reset (see R128InitCommonRegisters() and R128RestoreCommonRegisters() in r128_driver.c).  If R128PutImage() doesn't restore enough overlay state, you'll need to restart the overlay to get the right values back in the overlay regs.  radeon may have the same issue.
Comment 20 Matthias Hopf 2007-08-21 08:13:29 UTC
Stanislav, any updates from your side?
Otherwise I would suggest merging after 7.3 is out. The remaining issues don't sound sever.
Comment 21 Stanislav Brabec 2007-08-21 08:47:52 UTC
Please check in as is. It works and does most things one should expect and does not break anything.

If anybody can decide abot years-old bitrots in the code, I would be glad, but I am keeping all these commented-out parts of code there.

I plan to look at some remaining problems in future (but it's a low priority task). I could open separate bugs for them:

- Properly work after XRandR change.

- Properly support suspend/resume (not related to my patch).

- By one bug in brigthness settings. I can confirm it, but it has no practical impact. AFAIK Radeon driver has the same problem. Feel free to add a change proposed by you.

You wrote:

Your brightness + saturation controls are integer based, but don't have
rounding included. That may lead to strange effects, I suggest adding
half the denominator before doing the division - but of course you have
to check that no overflows can happen. BTW - brightness == 1000 will
already have the same representation (& 0x7f) as brightness == -1000...
Comment 22 Stanislav Brabec 2008-02-12 02:23:38 UTC
Created attachment 14281 [details] [review]
xf86-video-ati-6.7.197-r128-xvideo.patch

Patch ported to the latest git snapshot (replaces all three patches above).
Comment 23 Stanislav Brabec 2008-06-13 04:42:49 UTC
Ping. Is there any reason, why this patch was not yet accepted?

Comment #20 suggest to merge after 7.3. Minor fixes can come later.

The patch was already included in openSUSE 11.0.
Comment 24 Stefan Dirsch 2008-06-13 05:08:42 UTC
BTW, the patch no longer applies due to the driver split (ati --> radeon/r128/mach64).
Comment 25 Stanislav Brabec 2008-06-13 05:24:39 UTC
Updated patch from comment #22 applies on the current git://anongit.freedesktop.org/git/xorg/driver/xf86-video-r128.git without any reject.
Comment 26 Stefan Dirsch 2008-06-13 06:23:18 UTC
Well, it applies, but it doesn't build ...

r128_video.c:25:25: error: generic_bus.h: No such file or directory
r128_video.c:26:25: error: theatre_reg.h: No such file or directory
r128_video.c:27:21: error: theatre.h: No such file or directory
r128_video.c:28:28: error: theatre_detect.h: No such file or directory

... since these files are in seperate radeon driver tree.
Comment 27 Stanislav Brabec 2008-06-13 16:33:19 UTC
Created attachment 17108 [details] [review]
xf86-video-ati-theatre.patch

Additional patch to Comment #22. With this patch, the code compiles and works.

Before applying both these patches I did following:
git clone git://anongit.freedesktop.org/git/xorg/driver/xf86-video-r128.git
git clone git://anongit.freedesktop.org/git/xorg/driver/xf86-video-ati.git
cp -a xf86-video-ati/src/generic_bus.h xf86-video-r128/src/
cp -a xf86-video-ati/src/theatre_reg.h xf86-video-r128/src/
cp -a xf86-video-ati/src/theatre.h xf86-video-r128/src/
cp -a xf86-video-ati/src/theatre_detect.h xf86-video-r128/src/
cp -a xf86-video-ati/src/theatre_detect.c xf86-video-r128/src/
cp -a xf86-video-ati/src/theatre_detect_module.c xf86-video-r128/src/
cp -a xf86-video-ati/src/theatre.c xf86-video-r128/src/
cp -a xf86-video-ati/src/theatre_module.c xf86-video-r128/src/

Cloning sources seems to be sub-optimal (and causes file conflict), so I would suggest one of the following:

1) Keep the theatre code as is and move it to the directory with other multimedia chips in xserver. Edit radeon driver accordingly.

2) Clone files, rename radeon modules and its references in the r128 driver and remove support for ATI Rage Theatre 200 from them (AFAIK there was never manufactured any card combing ATI Rage 128 and ATI Rage Theatre 200).

What would you prefer?
Comment 28 Stefan Dirsch 2010-09-11 13:35:22 UTC
I've just added both patches to openSUSE's r128 driver (and an additional one to fix the switch to libpciaccess) + the copy action. Obviously nobody else is interested into TV-in support for All in Wonder 128. Hence closing as WONTFIX.

Internal information for Stanislav:

 47898  State:new     By:sndirsch     When:2010-09-11T22:20:23
        submit:       X11:XOrg/xorg-x11-driver-video  ->  openSUSE:Factory
        Descr: - xf86-video-ati-6.7.197-r128-xvideo.patch/
                 xf86-video-ati-theatre.patch
                 * readded TV-in support for All in Wonder 128 (bfo #8811)
               - xf86-video-ati-theatre-pciaccess.patch
                 * fixes build with libpciaccess
Comment 29 Stefan Dirsch 2010-09-11 13:36:59 UTC
Created attachment 38631 [details] [review]
xf86-video-ati-theatre-pciaccess.patch

Required since the switch to libpciaccess.

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.