Bug 10225

Summary: (Kind of) uninitialized variable, results in Xv bug on C&T 69030
Product: xorg Reporter: Petteri Aimonen <jpa>
Component: Driver/chipsAssignee: Egbert Eich <eich>
Status: RESOLVED FIXED QA Contact: Xorg Project Team <xorg-team>
Severity: normal    
Priority: medium CC: alexdeucher, jpa
Version: 7.2 (2007.02)Keywords: patch
Hardware: x86 (IA32)   
OS: Linux (All)   
URL: http://lxr.freedesktop.org/source/driver/xf86-video-chips/src/ct_video.c#L626
Whiteboard:
i915 platform: i915 features:

Description Petteri Aimonen 2007-03-08 12:26:36 UTC 
Line 641 of ct_video.c (see link), the tmp variable seems to be used somehow strangely.

This resulted in only top half of Xv video showing up when it used scaling, stretched to fill the area. At 1:1 size the video worked ok (as drw_h > src_h condition is false).

I rewrote the part based on the specs for 69030 (http://asiliant.com/pdf/ds182_3.pdf , search for "MR33"). The following code works for me, but I don't know about other chips cards.

I also haven't tested double scan modes (I don't know how they work). Spec says that the zoom factor is 0x100 / MR31, thus 0x100 / (0x100 * source / dest) = dest / source. Also added "& 0x7C" per the spec to keep MSB and bottom 2 bits clear, doesn't work otherwise.

Patch follows:

--- xf86-video-chips-1.1.1.orig/src/ct_video.c  2006-04-08 02:29:31.000000000 +0300
+++ xf86-video-chips-1.1.1/src/ct_video.c       2007-03-08 21:56:26.000000000 +0200
@@ -638,8 +638,8 @@
        if (dblscan)
            tmp = cPtr->VideoZoomMax >> 1;
        if (drw_h > src_h)
-           tmp = tmp * src_h / drw_h;
-       cPtr->writeMR(cPtr, 0x33, tmp);
+           tmp = 256 * src_h / drw_h;
+       cPtr->writeMR(cPtr, 0x33, tmp & 0x7C);
     }
     cPtr->writeMR(cPtr, 0x1F, m1f);
     cPtr->writeMR(cPtr, 0x1E, m1e);
Comment 1 Petteri Aimonen 2007-03-10 04:25:56 UTC 
Oops, bug in the masking. Only need to keep bottom two bits clear, there is no bit 8 :)

Correct patch follows:

--- xf86-video-chips-1.1.1.orig/src/ct_video.c  2006-04-08 02:29:31.000000000 +0300
+++ xf86-video-chips-1.1.1/src/ct_video.c       2007-03-10 14:18:48.000000000 +0200
@@ -638,8 +638,8 @@
        if (dblscan)
            tmp = cPtr->VideoZoomMax >> 1;
        if (drw_h > src_h)
-           tmp = tmp * src_h / drw_h;
-       cPtr->writeMR(cPtr, 0x33, tmp);
+           tmp = 256 * src_h / drw_h;
+       cPtr->writeMR(cPtr, 0x33, tmp & 0xFC);
     }
     cPtr->writeMR(cPtr, 0x1F, m1f);
     cPtr->writeMR(cPtr, 0x1E, m1e);
Comment 2 Alex Deucher 2007-03-16 17:41:15 UTC 
Committed:
242a378d6c0bead85b0b6e94f40c51849ba8f43b

Any chance you want to be the chips maintainer? :)
Comment 3 Petteri Aimonen 2007-03-16 23:55:52 UTC 
> Any chance you want to be the chips maintainer? :)

Hrm, too bad I don't usually have access to any chips based video card. As such, it would be difficult to do anything but apply patches from other people (which don't seem too ubiquitous :).

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.