intel-gtt: fix gtt_total_entries detection (e5e408fc) · Commits · 戴 / test

In commit Chris Wilson added some code to clear the full gtt on g33/pineview instead of just the mappable part. The code looks like it was copy-pasted from agp/intel-gtt.c, at least an identical piece of code is still there (in intel_i830_init_gtt_entries). This lead to a regression in 2.6.35 which was supposedly fixed in commit Now this commit makes absolutely no sense to me. It seems to be slightly confused about chipset generations - it references docs for 4th gen but the regression concerns 3rd gen g33. Luckily the the g33 gmch docs are available with the GMCH Graphics Control pci config register definitions. The other (bigger problem) is that the new check in there uses the i830 stolen mem bits (.5M, 1M or 8M of stolen mem). They are different since the i855GM. The most likely case is that it hits the 512M fallback, which was probably the right thing for the boxes this was tested on. So the original approach by Chris Wilson seems to be wrong and the current code is definitely wrong. There is a third approach by Jesse Barnes from his RFC patch "Who wants a bigger GTT mapping range?" where he simply shoves g33 in the same clause like later chipset generations. I've asked him and Jesse confirmed that this should work. So implement it. Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=16891$ Tested-by:

Anisse Astier <anisse@astier.eu> Cc: stable@kernel.org Signed-off-by:

Daniel Vetter <daniel.vetter@ffwll.ch> Signed-off-by:

Chris Wilson <chris@chris-wilson.co.uk>

drivers/char/agp/intel-gtt.c

+11 −39

Original line number	Diff line number	Diff line
		@@ -699,71 +699,43 @@ static unsigned int intel_gtt_stolen_entries(void)
		static unsigned int intel_gtt_total_entries(void)
		{
		int size;
		u16 gmch_ctrl;

		if (IS_I965) {
		if (IS_G33 \|\| IS_I965 \|\| IS_G4X) {
		u32 pgetbl_ctl;
		pgetbl_ctl = readl(intel_private.registers+I810_PGETBL_CTL);

		/* The 965 has a field telling us the size of the GTT,
		* which may be larger than what is necessary to map the
		* aperture.
		*/
		switch (pgetbl_ctl & I965_PGETBL_SIZE_MASK) {
		case I965_PGETBL_SIZE_128KB:
		size = 128;
		size = KB(128);
		break;
		case I965_PGETBL_SIZE_256KB:
		size = 256;
		size = KB(256);
		break;
		case I965_PGETBL_SIZE_512KB:
		size = 512;
		size = KB(512);
		break;
		case I965_PGETBL_SIZE_1MB:
		size = 1024;
		size = KB(1024);
		break;
		case I965_PGETBL_SIZE_2MB:
		size = 2048;
		size = KB(2048);
		break;
		case I965_PGETBL_SIZE_1_5MB:
		size = 1024 + 512;
		size = KB(1024 + 512);
		break;
		default:
		dev_info(&intel_private.pcidev->dev,
		"unknown page table size, assuming 512KB\n");
		size = 512;
		size = KB(512);
		}
		size += 4; /* add in BIOS popup space */
		} else if (IS_G33 && !IS_PINEVIEW) {
		/* G33's GTT size defined in gmch_ctrl */
		switch (gmch_ctrl & G33_PGETBL_SIZE_MASK) {
		case G33_PGETBL_SIZE_1M:
		size = 1024;
		break;
		case G33_PGETBL_SIZE_2M:
		size = 2048;
		break;
		default:
		dev_info(&intel_private.bridge_dev->dev,
		"unknown page table size 0x%x, assuming 512KB\n",
		(gmch_ctrl & G33_PGETBL_SIZE_MASK));
		size = 512;
		}
		size += 4;
		} else if (IS_G4X \|\| IS_PINEVIEW) {
		/* On 4 series hardware, GTT stolen is separate from graphics
		* stolen, ignore it in stolen gtt entries counting. However,
		* 4KB of the stolen memory doesn't get mapped to the GTT.
		*/
		size = 4;

		return size/4;
		} else {
		/* On previous hardware, the GTT size was just what was
		* required to map the aperture.
		*/
		size = agp_bridge->driver->fetch_size() + 4;
		return intel_private.base.gtt_mappable_entries;
		}

		return size/KB(4);
		}
		#endif

Admin message