i915 boot regression on Q35 chipset with 2.6.35-rc4

i915 boot regression on Q35 chipset with 2.6.35-rc4

[Tim Gardner]

[-- Attachment #1: Type: text/plain, Size: 363 bytes --]

Dave,

Please find attached a patch that fixes a boot time regression caused by 
commit f1befe71fa7a79ab733011b045639d8d809924ad. It has been reported to 
fix at least one user's boot problems.

I think it is also likely the source of the problems reported in 
http://marc.info/?l=linux-kernel&m=127869783002454&w=2

rtg
-- 
Tim Gardner tim.gardner@canonical.com

[-- Attachment #2: 0001-UBUNTU-Upstream-i915-Use-the-correct-mask-to-detect-.patch --]
[-- Type: text/x-patch, Size: 2179 bytes --]

>From e025bfb96059be648074a02f60dff92fb658cb9a Mon Sep 17 00:00:00 2001
From: Tim Gardner <tim.gardner@canonical.com>
Date: Fri, 9 Jul 2010 11:14:41 -0600
Subject: [PATCH] i915: Use the correct mask to detect i830 aperture size.

BugLink: https://bugs.launchpad.net/bugs/597075

commit f1befe71fa7a79ab733011b045639d8d809924ad introduced a regression when
detecting aperture size of some i915 adapters, e.g., those on the
Intel Q35 chipset.

The original report: https://bugzilla.kernel.org/show_bug.cgi?id=15733
The regression report: https://bugzilla.kernel.org/show_bug.cgi?id=16294

According to the specification found at http://intellinuxgraphics.org/VOL_1_graphics_core.pdf,
the PCI config space register I830_GMCH_CTRL is a mirror of GMCH Graphics Control. The correct
macro for isolating the aperture size bits is therefore I830_GMCH_GMS_MASK along with the
attendant changes to the case statement.

Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
Tested-by: Kees Cook <kees.cook@canonical.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Eric Anholt <eric@anholt.net>
Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/char/agp/intel-gtt.c |   13 ++++++++-----
 1 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/char/agp/intel-gtt.c b/drivers/char/agp/intel-gtt.c
index 9344216..a754715 100644
--- a/drivers/char/agp/intel-gtt.c
+++ b/drivers/char/agp/intel-gtt.c
@@ -1216,17 +1216,20 @@ static int intel_i915_get_gtt_size(void)
 
 		/* G33's GTT size defined in gmch_ctrl */
 		pci_read_config_word(agp_bridge->dev, I830_GMCH_CTRL, &gmch_ctrl);
-		switch (gmch_ctrl & G33_PGETBL_SIZE_MASK) {
-		case G33_PGETBL_SIZE_1M:
+		switch (gmch_ctrl & I830_GMCH_GMS_MASK) {
+		case I830_GMCH_GMS_STOLEN_512:
+			size = 512;
+			break;
+		case I830_GMCH_GMS_STOLEN_1024:
 			size = 1024;
 			break;
-		case G33_PGETBL_SIZE_2M:
-			size = 2048;
+		case I830_GMCH_GMS_STOLEN_8192:
+			size = 8*1024;
 			break;
 		default:
 			dev_info(&agp_bridge->dev->dev,
 				 "unknown page table size 0x%x, assuming 512KB\n",
-				(gmch_ctrl & G33_PGETBL_SIZE_MASK));
+				(gmch_ctrl & I830_GMCH_GMS_MASK));
 			size = 512;
 		}
 	} else {
-- 
1.7.0.4

Re: i915 boot regression on Q35 chipset with 2.6.35-rc4

[Woody Suwalski]

On Fri, Jul 9, 2010 at 4:48 PM, Tim Gardner <tim.gardner@canonical.com> wrote:
>
> Dave,
>
> Please find attached a patch that fixes a boot time regression caused by commit f1befe71fa7a79ab733011b045639d8d809924ad. It has been reported to fix at least one user's boot problems.
>
> I think it is also likely the source of the problems reported in http://marc.info/?l=linux-kernel&m=127869783002454&w=2
>
> rtg
> --
> Tim Gardner tim.gardner@canonical.com


Jesse, the above patch
0001-UBUNTU-Upstream-i915-Use-the-correct-mask-to-detect-.patch

is fixing my problem as well. No need for bisecting...

Thanks, Woody

PS. I am sending from GMail Web client - strange things (e.g .HTML)
may happen ;-(

re: i915 boot regression on Q35 chipset with 2.6.35-rc4

[Tim Gardner]

Dave,

http://lkml.org/lkml/2010/7/9/287

What's up with this patch? I haven't received any comment, nor has it 
appeared in your git repository.

rtg
-- 
Tim Gardner tim.gardner@canonical.com

re: i915 boot regression on Q35 chipset with 2.6.35-rc4

[Chris Wilson]

On Wed, 21 Jul 2010 14:35:22 +0200, Tim Gardner <tcanonical@tpi.com> wrote:
> What's up with this patch? I haven't received any comment, nor has it 
> appeared in your git repository.

Dave hasn't taken it because it is a regression fix for i915 and the
current procedure is for those to go through Eric's tree. Eric has been
busy working on a new compiler for GLSL the last few weeks and has been
applying patches in batches.

The patch I am happy to ack, but since I introduced the regression 
[https://bugzilla.kernel.org/show_bug.cgi?id=16294], I'd rather have
someone more familiar with the intricacies of G33/G35/Q35 review it.

For what is worth:
Acked-by: Chris Wilson <chris@chris-wilson.co.uk>

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: i915 boot regression on Q35 chipset with 2.6.35-rc4

[Tim Gardner]

On 07/21/2010 03:37 PM, Chris Wilson wrote:
> On Wed, 21 Jul 2010 14:35:22 +0200, Tim Gardner<tcanonical@tpi.com>  wrote:
>> What's up with this patch? I haven't received any comment, nor has it
>> appeared in your git repository.
>
> Dave hasn't taken it because it is a regression fix for i915 and the
> current procedure is for those to go through Eric's tree. Eric has been
> busy working on a new compiler for GLSL the last few weeks and has been
> applying patches in batches.
>
> The patch I am happy to ack, but since I introduced the regression
> [https://bugzilla.kernel.org/show_bug.cgi?id=16294], I'd rather have
> someone more familiar with the intricacies of G33/G35/Q35 review it.
>
> For what is worth:
> Acked-by: Chris Wilson<chris@chris-wilson.co.uk>
>

The events that reminded me to look up the status of this patch were the 
i915 regression fixes that recently appeared in 
git-commits-head@vger.kernel.org from Dave's tree (which appears to be 
contrary to the current procedure mentioned above).

rtg
-- 
Tim Gardner tim.gardner@canonical.com

re: i915 boot regression on Q35 chipset with 2.6.35-rc4

[Dave Airlie]

On Wed, 2010-07-21 at 14:35 +0200, Tim Gardner wrote:
> Dave,
> 
> http://lkml.org/lkml/2010/7/9/287
> 
> What's up with this patch? I haven't received any comment, nor has it 
> appeared in your git repository.

I don't deal with Intel patches directly, Eric?

Dave.

RE: i915 boot regression on Q35 chipset with 2.6.35-rc4

[Jin, Gordon]

Chris Wilson wrote on Wednesday, July 21, 2010 9:37 PM:
> On Wed, 21 Jul 2010 14:35:22 +0200, Tim Gardner <tcanonical@tpi.com>
> wrote: 
>> What's up with this patch? I haven't received any comment, nor has it
>> appeared in your git repository.
> 
> Dave hasn't taken it because it is a regression fix for i915 and the
> current procedure is for those to go through Eric's tree. Eric has
> been busy working on a new compiler for GLSL the last few weeks and
> has been applying patches in batches.
> 
> The patch I am happy to ack, but since I introduced the regression
> [https://bugzilla.kernel.org/show_bug.cgi?id=16294], I'd rather have
> someone more familiar with the intricacies of G33/G35/Q35 review it.
> 
> For what is worth:
> Acked-by: Chris Wilson <chris@chris-wilson.co.uk>

This patch fixes the bug on our G33 (0x29b2).
Tested-by: Xudong Hao <xudong.hao@intel.com>

-Gordon

Re: i915 boot regression on Q35 chipset with 2.6.35-rc4

[Jiri Slaby]

On 07/22/2010 11:19 AM, Jin, Gordon wrote:
> Chris Wilson wrote on Wednesday, July 21, 2010 9:37 PM:
>> On Wed, 21 Jul 2010 14:35:22 +0200, Tim Gardner <tcanonical@tpi.com>
>> wrote: 
>>> What's up with this patch? I haven't received any comment, nor has it
>>> appeared in your git repository.
>>
>> Dave hasn't taken it because it is a regression fix for i915 and the
>> current procedure is for those to go through Eric's tree. Eric has
>> been busy working on a new compiler for GLSL the last few weeks and
>> has been applying patches in batches.
>>
>> The patch I am happy to ack, but since I introduced the regression
>> [https://bugzilla.kernel.org/show_bug.cgi?id=16294], I'd rather have
>> someone more familiar with the intricacies of G33/G35/Q35 review it.
>>
>> For what is worth:
>> Acked-by: Chris Wilson <chris@chris-wilson.co.uk>
> 
> This patch fixes the bug on our G33 (0x29b2).

FWIW works for me too.

And as this is present as of 35-rc1 upstream, this means almost 2 months
lag since the break till now. Could you guys sort this up ASAP, please?

thanks,
-- 
js

RE: i915 boot regression on Q35 chipset with 2.6.35-rc4

[Jin, Gordon]

Jin, Gordon wrote on Thursday, July 22, 2010 5:19 PM:
> Chris Wilson wrote on Wednesday, July 21, 2010 9:37 PM:
>> On Wed, 21 Jul 2010 14:35:22 +0200, Tim Gardner <tcanonical@tpi.com>
>> wrote:
>>> What's up with this patch? I haven't received any comment, nor has
>>> it appeared in your git repository.
>> 
>> Dave hasn't taken it because it is a regression fix for i915 and the
>> current procedure is for those to go through Eric's tree. Eric has
>> been busy working on a new compiler for GLSL the last few weeks and
>> has been applying patches in batches.
>> 
>> The patch I am happy to ack, but since I introduced the regression
>> [https://bugzilla.kernel.org/show_bug.cgi?id=16294], I'd rather have
>> someone more familiar with the intricacies of G33/G35/Q35 review it.
>> 
>> For what is worth:
>> Acked-by: Chris Wilson <chris@chris-wilson.co.uk>
> 
> This patch fixes the bug on our G33 (0x29b2).
> Tested-by: Xudong Hao <xudong.hao@intel.com>

Sorry, the pci id for above G33 is actually 0x29c2.

And we also validated the patch on Q35 (0x29b2):
Tested-by: Xun Fang <xunx.fang@intel.com>

-Gordon

Re: i915 boot regression on Q35 chipset with 2.6.35-rc4

[Eric Anholt]

[-- Attachment #1: Type: text/plain, Size: 449 bytes --]

On Fri, 09 Jul 2010 14:48:50 -0600, Tim Gardner <tim.gardner@canonical.com> wrote:
> Dave,
> 
> Please find attached a patch that fixes a boot time regression caused by 
> commit f1befe71fa7a79ab733011b045639d8d809924ad. It has been reported to 
> fix at least one user's boot problems.
> 
> I think it is also likely the source of the problems reported in 
> http://marc.info/?l=linux-kernel&m=127869783002454&w=2

Applied to for-linus.

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]