intel-agp: Avoid oops for G33 on 1MB stolen case
PATCH 1/1 intel-agp: Avoid oops for G33 on 1MB stolen case
This is similar to f443675affe3f16dd428e46f0f7fd3f4d703eeab which was reverted because it broke older X.org driver. This patch only fixes the 1MB stolen case since it causes an oops.
Xorg will not work without the accompanying patch[1] but avoiding an oops and making it possible to work with patched xorg driver is reasonable.
[1] http://ifup.org/~philips/review/xf86-video-intel-G33-1mb.patch
Explanation of the oops:
Quoted 1 line
static void
intel_i830_init_gtt_entries(void)
...
Quoted 6 lines
} else if (IS_G33) {
/* G33's GTT size defined in gmch_ctrl */
switch (gmch_ctrl & G33_PGETBL_SIZE_MASK) {
case G33_PGETBL_SIZE_1M:
size = 1024;
break;...
Quoted 1 line
size += 4;
size = 1028
Then since we have the BIOS setting 1MB for the device in the GMCH control we get to here:
Quoted 5 lines
} else {
switch (gmch_ctrl & I855_GMCH_GMS_MASK) {
case I855_GMCH_GMS_STOLEN_1M:
gtt_entries = MB(1) - KB(size);
break;MB(1) = 1 * 1024 * 1024
KB(1028) = 1028 * 1024
MB(1) - KB(1028) = -4096
Quoted 2 lines
gtt_entries /= KB(4);
intel_private.gtt_entries = gtt_entries;We end up with -1 in gtt_entries.
This leads to intel_i915_configure reading/writing to areas outside of mapped memory and the oops.
Bugzilla: https://bugzilla.novell.com/show_bug.cgi?id=391261
--- drivers/char/agp/intel-agp.c | 8 ++++++++ 1 file changed, 8 insertions(+)
drivers/char/agp/intel-agp.c +8 -0
@@ -559,6 +559,14 @@ static void intel_i830_init_gtt_entries( }else {switch (gmch_ctrl &I855_GMCH_GMS_MASK ) {case I855_GMCH_GMS_STOLEN_1M : +if (IS_G33 ) { + size =0 ; + printk(KERN_WARNING PFX +"Warning: G33 chipset with 1MB" +" allocated. Older X.org Intel drivers" + " will not work.\n"); +WARN_ON (1 ); + } gtt_entries =MB (1 ) -KB (size);break ;case I855_GMCH_GMS_STOLEN_4M :
Quoted 64 lines · On Tue, 14 Oct 2008 15:58:17 -0700 Brandon Philips <[email protected]> wrote:
This is similar to
f443675affe3f16dd428e46f0f7fd3f4d703eeabwhich was reverted because it broke older X.org driver. This patch only fixes the 1MB stolen case since it causes an oops.Xorg will not work without the accompanying patch[1] but avoiding an oops and making it possible to work with patched xorg driver is reasonable.
[1] http://ifup.org/~philips/review/xf86-video-intel-G33-1mb.patch
Explanation of the oops:
Quoted 1 line
static void
intel_i830_init_gtt_entries(void)...
Quoted 6 lines
} else if (IS_G33) { /* G33's GTT size defined in gmch_ctrl */ switch (gmch_ctrl & G33_PGETBL_SIZE_MASK) { case G33_PGETBL_SIZE_1M: size = 1024; break;...
Quoted 1 line
size += 4;size = 1028
Then since we have the BIOS setting 1MB for the device in the GMCH control we get to here:
Quoted 5 lines
} else { switch (gmch_ctrl & I855_GMCH_GMS_MASK) { case I855_GMCH_GMS_STOLEN_1M: gtt_entries = MB(1) - KB(size); break;MB(1) = 1 * 1024 * 1024
KB(1028) = 1028 * 1024MB(1) - KB(1028) = -4096
Quoted 2 lines
gtt_entries /= KB(4); intel_private.gtt_entries = gtt_entries;We end up with -1 in
gtt_entries.This leads to
intel_i915_configurereading/writing to areas outside of mapped memory and the oops.Bugzilla: https://bugzilla.novell.com/show_bug.cgi?id=391261
Signed-off-by Brandon Philips <[email protected]>--- drivers/char/agp/intel-agp.c | 8 ++++++++ 1 file changed, 8 insertions(+)Index:
linux-2.6/drivers/char/agp/intel-agp.c
===================================================================drivers/char/agp/intel-agp.c +1
-0@@ -559,6 +559,14 @@ static void intel_i830_init_gtt_entries( }else {switch (gmch_ctrl &I855_GMCH_GMS_MASK ) {case I855_GMCH_GMS_STOLEN_1M : +if (IS_G33 ) {
drivers/char/agp/intel-agp.c static void intel_i830_init_gtt_entries( · line 572
Quoted 10 lines
drivers/char/agp/intel-agp.c +7
-0+ size =0 ; + printk(KERN_WARNING PFX +"Warning: G33 chipset with 1MB" +" allocated. Older X.org Intel drivers" + " will not work.\n"); +WARN_ON (1 ); + } gtt_entries =MB (1 ) -KB (size);break ;case I855_GMCH_GMS_STOLEN_4M :
Is the bug which this patch addresses present in the 2.6.27 kernel?
Thanks.
Dave Airlie
maintainerQuoted 71 lines · Andrew Morton
Quoted 68 lines · On Tue, 14 Oct 2008 15:58:17 -0700 Brandon Philips <[email protected]> wrote:
This is similar to
f443675affe3f16dd428e46f0f7fd3f4d703eeabwhich was reverted because it broke older X.org driver. This patch only fixes the 1MB stolen case since it causes an oops.Xorg will not work without the accompanying patch[1] but avoiding an oops and making it possible to work with patched xorg driver is reasonable.
[1] http://ifup.org/~philips/review/xf86-video-intel-G33-1mb.patch
Explanation of the oops:
Quoted 1 line
static void
intel_i830_init_gtt_entries(void)...
Quoted 6 lines
} else if (IS_G33) { /* G33's GTT size defined in gmch_ctrl */ switch (gmch_ctrl & G33_PGETBL_SIZE_MASK) { case G33_PGETBL_SIZE_1M: size = 1024; break;...
Quoted 1 line
size += 4;size = 1028
Then since we have the BIOS setting 1MB for the device in the GMCH control we get to here:
Quoted 5 lines
} else { switch (gmch_ctrl & I855_GMCH_GMS_MASK) { case I855_GMCH_GMS_STOLEN_1M: gtt_entries = MB(1) - KB(size); break;MB(1) = 1 * 1024 * 1024
KB(1028) = 1028 * 1024MB(1) - KB(1028) = -4096
Quoted 2 lines
gtt_entries /= KB(4); intel_private.gtt_entries = gtt_entries;We end up with -1 in
gtt_entries.This leads to
intel_i915_configurereading/writing to areas outside of mapped memory and the oops.Bugzilla: https://bugzilla.novell.com/show_bug.cgi?id=391261
Signed-off-by Brandon Philips <[email protected]>--- drivers/char/agp/intel-agp.c | 8 ++++++++ 1 file changed, 8 insertions(+)Index:
linux-2.6/drivers/char/agp/intel-agp.c
===================================================================drivers/char/agp/intel-agp.c +5
-0@@ -559,6 +559,14 @@ static void intel_i830_init_gtt_entries( }else {switch (gmch_ctrl &I855_GMCH_GMS_MASK ) {case I855_GMCH_GMS_STOLEN_1M : +if (IS_G33 ) { + size =0 ; + printk(KERN_WARNING PFX +"Warning: G33 chipset with 1MB" +" allocated. Older X.org Intel drivers"
Quoted 9 lines
drivers/char/agp/intel-agp.c static void intel_i830_init_gtt_entries( · line 572
Quoted 6 lines
drivers/char/agp/intel-agp.c +3
-0+ " will not work.\n"); +WARN_ON (1 ); + } gtt_entries =MB (1 ) -KB (size);break ;case I855_GMCH_GMS_STOLEN_4M :Is the bug which this patch addresses present in the 2.6.27 kernel?
I've been a bit wary about this patch, but on re-review I suppose it should be fine. We'll just get WARN_ONs in places we don't really want them, and I'm sure Arjan will come complaining about them from kerneloops.
However as I said at KS there is no nice answer to this, we are hopelessly screwed.
Dave.
Quoted 15 lines · On Tue, 21 Oct 2008 07:44:46 +1000 Dave Airlie <[email protected]> wrote:
Quoted 5 lines
drivers/char/agp/intel-agp.c +1
-0+ printk(KERN_WARNING PFXQuoted 4 lines
(quoted diff) +1
-0+ "Warning: G33 chipsetwith 1MB"
(quoted diff) +1
-0+ " allocated. Older X.orgIntel drivers"
Quoted 9 lines
drivers/char/agp/intel-agp.c static void intel_i830_init_gtt_entries( · line 572
Quoted 6 lines
drivers/char/agp/intel-agp.c +3
-0+ " will not work.\n"); +WARN_ON (1 ); + } gtt_entries =MB (1 ) -KB (size);break ;case I855_GMCH_GMS_STOLEN_4M :Is the bug which this patch addresses present in the 2.6.27 kernel?
Quoted 5 lines
I've been a bit wary about this patch, but on re-review I suppose it should be fine. We'll just get
WARN_ONsin places we don't really want them, and I'm sure Arjan will come complaining about them from kerneloops.
the patch is not so nice for this;
the printk+WARN_ON() really should be using WARN() instead; that also allows me to filter these guys out easily if needed.
Signature · 3 lines
Arjan van de Ven Intel Open Source Technology Centre For development, discussion and tips for power savings, visit http://www.lesswatts.org
Quoted 17 lines · Arjan van de Ven
Quoted 15 lines · On Tue, 21 Oct 2008 07:44:46 +1000 Dave Airlie <[email protected]> wrote:
Quoted 14 lines
drivers/char/agp/intel-agp.c +1
-0+ printk(KERN_WARNING PFXdrivers/char/agp/intel-agp.c static void intel_i830_init_gtt_entries( · line 572
Quoted 10 lines
(quoted diff) +1
-0+ "Warning: G33 chipsetwith 1MB"
(quoted diff) +1
-0+ " allocated. Older X.orgIntel drivers"
drivers/char/agp/intel-agp.c +3
-0+ " will not work.\n"); +WARN_ON (1 ); + } gtt_entries =MB (1 ) -KB (size);break ;case I855_GMCH_GMS_STOLEN_4M :Is the bug which this patch addresses present in the 2.6.27 kernel?
Quoted 4 lines
Quoted 4 lines
I've been a bit wary about this patch, but on re-review I suppose it should be fine. We'll just get
WARN_ONsin places we don't really want them, and I'm sure Arjan will come complaining about them from kerneloops.
I figured it was better to be noisy than to have people complaining about X.org being broken.
Should it be something other than a WARN?
Quoted 3 lines
the patch is not so nice for this;
the printk+WARN_ON() really should be usingWARN()instead; that also allows me to filter these guys out easily if needed.
Ok. I will send a new patch in a moment using WARN().
Thanks,
Brandon
PATCH v2 1/1 intel-agp: Avoid oops for G33 on 1MB stolen case
In reply to Arjan van de VenThis is similar to f443675affe3f16dd428e46f0f7fd3f4d703eeab which was reverted because it broke older X.org driver. This patch only fixes the 1MB stolen case since it causes an oops.
Xorg will not work without the accompanying patch[1] but avoiding an oops and making it possible to work with patched xorg driver is reasonable.
[1] http://ifup.org/~philips/review/xf86-video-intel-G33-1mb.patch
Explanation of the oops:
Quoted 1 line
static void
intel_i830_init_gtt_entries(void)
...
Quoted 6 lines
} else if (IS_G33) {
/* G33's GTT size defined in gmch_ctrl */
switch (gmch_ctrl & G33_PGETBL_SIZE_MASK) {
case G33_PGETBL_SIZE_1M:
size = 1024;
break;...
Quoted 1 line
size += 4;
size = 1028
Then since we have the BIOS setting 1MB for the device in the GMCH control we get to here:
Quoted 5 lines
} else {
switch (gmch_ctrl & I855_GMCH_GMS_MASK) {
case I855_GMCH_GMS_STOLEN_1M:
gtt_entries = MB(1) - KB(size);
break;MB(1) = 1 * 1024 * 1024
KB(1028) = 1028 * 1024
MB(1) - KB(1028) = -4096
Quoted 2 lines
gtt_entries /= KB(4);
intel_private.gtt_entries = gtt_entries;We end up with -1 in gtt_entries.
This leads to intel_i915_configure reading/writing to areas outside of mapped memory and the oops.
Bugzilla: https://bugzilla.novell.com/show_bug.cgi?id=391261
--- drivers/char/agp/intel-agp.c | 7 +++++++ 1 file changed, 7 insertions(+)
drivers/char/agp/intel-agp.c +7 -0
@@ -561,6 +561,13 @@ static void intel_i830_init_gtt_entries( }else {switch (gmch_ctrl &I855_GMCH_GMS_MASK ) {case I855_GMCH_GMS_STOLEN_1M : +if (IS_G33 ) { + size =0 ; +WARN (1 ,KERN_WARNING +"Warning: G33 chip with 1MB allocated." +" Older X.org Intel drivers will not" +" work.\n" ); + } gtt_entries =MB (1 ) -KB (size);break ;case I855_GMCH_GMS_STOLEN_4M :
Quoted 71 lines · Andrew Morton
Quoted 68 lines · On Tue, 14 Oct 2008 15:58:17 -0700 Brandon Philips <[email protected]> wrote:
This is similar to
f443675affe3f16dd428e46f0f7fd3f4d703eeabwhich was reverted because it broke older X.org driver. This patch only fixes the 1MB stolen case since it causes an oops.Xorg will not work without the accompanying patch[1] but avoiding an oops and making it possible to work with patched xorg driver is reasonable.
[1] http://ifup.org/~philips/review/xf86-video-intel-G33-1mb.patch
Explanation of the oops:
Quoted 1 line
static void
intel_i830_init_gtt_entries(void)...
Quoted 6 lines
} else if (IS_G33) { /* G33's GTT size defined in gmch_ctrl */ switch (gmch_ctrl & G33_PGETBL_SIZE_MASK) { case G33_PGETBL_SIZE_1M: size = 1024; break;...
Quoted 1 line
size += 4;size = 1028
Then since we have the BIOS setting 1MB for the device in the GMCH control we get to here:
Quoted 5 lines
} else { switch (gmch_ctrl & I855_GMCH_GMS_MASK) { case I855_GMCH_GMS_STOLEN_1M: gtt_entries = MB(1) - KB(size); break;MB(1) = 1 * 1024 * 1024
KB(1028) = 1028 * 1024MB(1) - KB(1028) = -4096
Quoted 2 lines
gtt_entries /= KB(4); intel_private.gtt_entries = gtt_entries;We end up with -1 in
gtt_entries.This leads to
intel_i915_configurereading/writing to areas outside of mapped memory and the oops.Bugzilla: https://bugzilla.novell.com/show_bug.cgi?id=391261
Signed-off-by Brandon Philips <[email protected]>--- drivers/char/agp/intel-agp.c | 8 ++++++++ 1 file changed, 8 insertions(+)Index:
linux-2.6/drivers/char/agp/intel-agp.c
===================================================================drivers/char/agp/intel-agp.c +5
-0@@ -559,6 +559,14 @@ static void intel_i830_init_gtt_entries( }else {switch (gmch_ctrl &I855_GMCH_GMS_MASK ) {case I855_GMCH_GMS_STOLEN_1M : +if (IS_G33 ) { + size =0 ; + printk(KERN_WARNING PFX +"Warning: G33 chipset with 1MB" +" allocated. Older X.org Intel drivers"
Quoted 8 lines
drivers/char/agp/intel-agp.c static void intel_i830_init_gtt_entries( · line 572
Quoted 6 lines
drivers/char/agp/intel-agp.c +3
-0+ " will not work.\n"); +WARN_ON (1 ); + } gtt_entries =MB (1 ) -KB (size);break ;case I855_GMCH_GMS_STOLEN_4M :Is the bug which this patch addresses present in the 2.6.27 kernel?
Yes, ever since 3a4986955c0d9806e96a9d738ef7c40cb0cdaba3
Thanks,
Brandon