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

Proposed by Nick Andrik
Status: Rejected
Rejected by: Martin Pitt
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 Disapprove
penalvch (community) suitability Abstain
Ubuntu-X Pending
Ubuntu branches 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.
Revision history for this message
penalvch (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

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

Revision history for this message
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

Revision history for this message
penalvch (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)
Revision history for this message
Timo Aaltonen (tjaalton) wrote :

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

review: Disapprove
Revision history for this message
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

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

176. By Nick Andrik

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

175. By Nick Andrik

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

174. By Nick Andrik

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

Subscribers

People subscribed via source and target branches

to all changes: