Merge lp:~andrikos/ubuntu/quantal/xserver-xorg-video-intel/fix-ati-hybrid into lp:ubuntu/quantal/xserver-xorg-video-intel

Proposed by Nick Andrik on 2012-11-05
Status: Rejected
Rejected by: Martin Pitt on 2012-12-05
Proposed branch: lp:~andrikos/ubuntu/quantal/xserver-xorg-video-intel/fix-ati-hybrid
Merge into: lp:ubuntu/quantal/xserver-xorg-video-intel
Diff against target: 181 lines (+163/-0)
3 files modified
debian/changelog (+7/-0)
debian/patches/fix-ati-hybrid.patch (+155/-0)
debian/patches/series (+1/-0)
To merge this branch: bzr merge lp:~andrikos/ubuntu/quantal/xserver-xorg-video-intel/fix-ati-hybrid
Reviewer Review Type Date Requested Status
Timo Aaltonen 2012-11-14 Disapprove on 2012-12-05
Christopher M. Penalver (community) suitability Abstain on 2012-11-06
Ubuntu-X 2012-11-14 Pending
Ubuntu branches 2012-11-05 Pending
Review via email: mp+132956@code.launchpad.net

Description of the Change

This update includes a patch to revert a speicific commit of the upstream intel driver that breaks ati/intel hybrid systems.

To post a comment you must log in.
Christopher M. Penalver (penalvch) wrote :

This patch, based on Nick's PPA, has not been shown to work for the original reporter as per http://ati.cchtml.com/show_bug.cgi?id=624 .

Review -> Disapprove

review: Disapprove (suitability)
175. By Nick Andrik on 2012-11-05

Removing reference to (LP: #1068661) since it is not a confirmed duplicate

Nick Andrik (andrikos) wrote :

This patch fixes #1068404 and this is where its targeted.

The bug on http://ati.cchtml.com/show_bug.cgi?id=624 refers to #1068661 which (although it may seem) it is not a confirmed duplicate

Christopher M. Penalver (penalvch) wrote :

The previously mentioned Review: Disapprove was assessed for a different bug report https://bugs.launchpad.net/fglrx/+bug/1068661 . However, because the same branch was linked to two different bug reports, my intending Disapprove for only one report, flowed to both.

Review -> Abstain

review: Abstain (suitability)
Timo Aaltonen (tjaalton) wrote :

upstream is right, and we shouldn't work around bugs in binary drivers

review: Disapprove
Nick Andrik (andrikos) wrote :

By using the mentioned patch we enable people to use hybrid intel/fglrx systems with no (at least from my point of view, correct me if I am wrong) disadvantage at all.

There are two official packages in the repositories, i.e. xserver-xorg-video-intel and fglrx(-updates), that make the intel/ATI hybrid systems completely unusable when the two of them are used together.

This issue has to be addressed one why or another:
- My proposal is to revert that specific xorg-intel commit (and use the patch at least in ubuntu, if upstream does not want to include it).
- If we decide not to do that, we should include a "Conflicts:" dependency in the packages, and warn/disallow people to use the fglrx driver when the xserver-xorg-video-intel is also used

In either case, something has to be done. This is a real problem and we cannot just ignore it, let people shoot themselves in the foot and blame ubuntu for fglrx drivers.

Just my 2ยข

Unmerged revisions

177. By Nick Andrik on 2013-01-04

- Updated package version to mark NMU for SRU
- Removed reference to (LP: #1068661) since it is now a confirmed duplicate

176. By Nick Andrik on 2012-12-19

Added (LP: #1068404) in the list of the bugs fixed by this patch

175. By Nick Andrik on 2012-11-05

Removing reference to (LP: #1068661) since it is not a confirmed duplicate

174. By Nick Andrik on 2012-11-05

Add fix-ati-hybrid.patch to fix ATI/Intel hybrid setups
(LP: #1068404, #1068661)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'debian/changelog'
2--- debian/changelog 2012-10-01 15:39:54 +0000
3+++ debian/changelog 2012-11-05 20:59:20 +0000
4@@ -1,3 +1,10 @@
5+xserver-xorg-video-intel (2:2.20.9-0ubuntu2+andrik1) quantal; urgency=low
6+
7+ * Add fix-ati-hybrid.patch to fix ATI/Intel hybrid setups
8+ (LP: #1068661)
9+
10+ -- Nick Andrik <nick.andrik@gmail.com> Mon, 05 Nov 2012 21:54:26 +0100
11+
12 xserver-xorg-video-intel (2:2.20.9-0ubuntu2) quantal; urgency=low
13
14 [ Maarten Lankhorst ]
15
16=== added file 'debian/patches/fix-ati-hybrid.patch'
17--- debian/patches/fix-ati-hybrid.patch 1970-01-01 00:00:00 +0000
18+++ debian/patches/fix-ati-hybrid.patch 2012-11-05 20:59:20 +0000
19@@ -0,0 +1,155 @@
20+Fixes regression for ati/intel hybrid systems
21+by reverting some commits of xf86-video-intel:
22+
23+ commit 05dcc5f1699ba90fc14c50882e8d4be89bc4a4f9
24+ Author: Chris Wilson <chris@chris-wilson.co.uk>
25+ Date: Fri Aug 3 15:08:45 2012 +0100
26+
27+ Pass the chipset info through driverPrivate rather than a global pointer
28+
29+
30+Nick Andrik <nick.andrik@gmail.com>
31+Index: xserver-xorg-video-intel-2.20.9/src/intel_driver.c
32+===================================================================
33+--- xserver-xorg-video-intel-2.20.9.orig/src/intel_driver.c 2012-10-30 02:54:05.456713161 +0100
34++++ xserver-xorg-video-intel-2.20.9/src/intel_driver.c 2012-10-30 02:54:16.186075606 +0100
35+@@ -185,7 +185,7 @@
36+ static void intel_check_chipset_option(ScrnInfoPtr scrn)
37+ {
38+ intel_screen_private *intel = intel_get_screen_private(scrn);
39+- intel_detect_chipset(scrn, intel->pEnt, intel->PciInfo);
40++ intel->info = intel_detect_chipset(scrn, intel->pEnt, intel->PciInfo);
41+ }
42+
43+ static Bool I830GetEarlyOptions(ScrnInfoPtr scrn)
44+@@ -491,15 +491,14 @@
45+ if (flags & PROBE_DETECT)
46+ return TRUE;
47+
48+- if (((uintptr_t)scrn->driverPrivate) & 1) {
49+- intel = xnfcalloc(sizeof(*intel), 1);
50++ intel = intel_get_screen_private(scrn);
51++ if (intel == NULL) {
52++ intel = xnfcalloc(sizeof(intel_screen_private), 1);
53+ if (intel == NULL)
54+ return FALSE;
55+
56+- intel->info = (void *)((uintptr_t)scrn->driverPrivate & ~1);
57+ scrn->driverPrivate = intel;
58+ }
59+- intel = intel_get_screen_private(scrn);
60+ intel->scrn = scrn;
61+ intel->pEnt = pEnt;
62+
63+Index: xserver-xorg-video-intel-2.20.9/src/intel_driver.h
64+===================================================================
65+--- xserver-xorg-video-intel-2.20.9.orig/src/intel_driver.h 2012-10-30 02:54:05.456713161 +0100
66++++ xserver-xorg-video-intel-2.20.9/src/intel_driver.h 2012-10-30 02:54:16.186075606 +0100
67+@@ -277,9 +277,9 @@
68+ int gen;
69+ };
70+
71+-void intel_detect_chipset(ScrnInfoPtr scrn,
72+- EntityInfoPtr ent,
73+- struct pci_device *pci);
74++const struct intel_device_info *
75++intel_detect_chipset(ScrnInfoPtr scrn,
76++ EntityInfoPtr ent, struct pci_device *pci);
77+
78+
79+ #endif /* INTEL_DRIVER_H */
80+Index: xserver-xorg-video-intel-2.20.9/src/intel_module.c
81+===================================================================
82+--- xserver-xorg-video-intel-2.20.9.orig/src/intel_module.c 2012-10-30 02:54:05.456713161 +0100
83++++ xserver-xorg-video-intel-2.20.9/src/intel_module.c 2012-10-30 02:54:16.186075606 +0100
84+@@ -51,6 +51,8 @@
85+ #include <xf86platformBus.h>
86+ #endif
87+
88++static struct intel_device_info *chipset_info;
89++
90+ static const struct intel_device_info intel_generic_info = {
91+ .gen = -1,
92+ };
93+@@ -315,10 +317,9 @@
94+ { 0, 0, 0 },
95+ };
96+
97+-void
98++const struct intel_device_info *
99+ intel_detect_chipset(ScrnInfoPtr scrn,
100+- EntityInfoPtr ent,
101+- struct pci_device *pci)
102++ EntityInfoPtr ent, struct pci_device *pci)
103+ {
104+ MessageType from = X_PROBED;
105+ const char *name = NULL;
106+@@ -347,6 +348,7 @@
107+ }
108+
109+ scrn->chipset = name;
110++ return chipset_info;
111+ }
112+
113+ /*
114+@@ -481,6 +483,8 @@
115+ PciChipsets intel_pci_chipsets[NUM_CHIPSETS];
116+ unsigned i;
117+
118++ chipset_info = (void *)match_data;
119++
120+ if (!has_kernel_mode_setting(device)) {
121+ #if KMS_ONLY
122+ return FALSE;
123+@@ -515,7 +519,6 @@
124+ scrn->driverVersion = INTEL_VERSION;
125+ scrn->driverName = INTEL_DRIVER_NAME;
126+ scrn->name = INTEL_NAME;
127+- scrn->driverPrivate = (void *)(match_data | 1);
128+ scrn->Probe = NULL;
129+
130+ #if !KMS_ONLY
131+Index: xserver-xorg-video-intel-2.20.9/src/legacy/i810/i810_driver.c
132+===================================================================
133+--- xserver-xorg-video-intel-2.20.9.orig/src/legacy/i810/i810_driver.c 2012-10-30 02:54:05.456713161 +0100
134++++ xserver-xorg-video-intel-2.20.9/src/legacy/i810/i810_driver.c 2012-10-30 02:54:16.186075606 +0100
135+@@ -151,7 +151,7 @@
136+ static Bool
137+ I810GetRec(ScrnInfoPtr scrn)
138+ {
139+- if (((uintptr_t)scrn->driverPrivate & 1) == 0)
140++ if (scrn->driverPrivate)
141+ return TRUE;
142+
143+ scrn->driverPrivate = xnfcalloc(sizeof(I810Rec), 1);
144+Index: xserver-xorg-video-intel-2.20.9/src/sna/sna_driver.c
145+===================================================================
146+--- xserver-xorg-video-intel-2.20.9.orig/src/sna/sna_driver.c 2012-10-30 02:54:05.456713161 +0100
147++++ xserver-xorg-video-intel-2.20.9/src/sna/sna_driver.c 2012-10-30 02:54:16.190077605 +0100
148+@@ -407,15 +407,14 @@
149+
150+ sna_selftest();
151+
152+- if (((uintptr_t)scrn->driverPrivate) & 1) {
153++ sna = to_sna(scrn);
154++ if (sna == NULL) {
155+ sna = xnfcalloc(sizeof(struct sna), 1);
156+ if (sna == NULL)
157+ return FALSE;
158+
159+- sna->info = (void *)((uintptr_t)scrn->driverPrivate & ~1);
160+ scrn->driverPrivate = sna;
161+ }
162+- sna = to_sna(scrn);
163+ sna->scrn = scrn;
164+ sna->pEnt = pEnt;
165+
166+@@ -465,7 +464,7 @@
167+
168+ sna_setup_capabilities(scrn, fd);
169+
170+- intel_detect_chipset(scrn, sna->pEnt, sna->PciInfo);
171++ sna->info = intel_detect_chipset(scrn, sna->pEnt, sna->PciInfo);
172+
173+ kgem_init(&sna->kgem, fd, sna->PciInfo, sna->info->gen);
174+ if (xf86ReturnOptValBool(sna->Options, OPTION_ACCEL_DISABLE, FALSE)) {
175
176=== modified file 'debian/patches/series'
177--- debian/patches/series 2012-10-01 15:39:54 +0000
178+++ debian/patches/series 2012-11-05 20:59:20 +0000
179@@ -1,1 +1,2 @@
180 fix-glamor-check.diff
181+fix-ati-hybrid.patch

Subscribers

People subscribed via source and target branches

to all changes: