[PATCH 0/8] Gen2 stolen/local memory support

[PATCH 0/8] Gen2 stolen/local memory support

[ville.syrjala]

While poking around with FBC, I noticed that we don't currently have
stolen memory support for gen2. This series aims to fix that.

I also tried to add support for 830M local memory. Now, I don't actually
know if there are systems out there equipped with local memory or not. But
if they do exist, I wouldn't mind getting one...

As I only have a single gen2 system (855GM), this has only received limited
testing.

Ville Syrjälä (8):
      x86: Add vfunc for Intel graphics stolen memory base address
      x86: Add Intel graphics stolen memory quirk for gen2 platforms
      intel-gtt: Return whether we have local memory or not
      intel-gtt: Assume last 128KB of stolen contains the GTT entries on gen2
      intel-gtt: Use i810_write_entry() on gen2 platforms
      drm/i915: Add I915_CACHE_LOCAL to indicate local memory
      drm/i915: Keep track if we have local memory
      drm/i915: Determine the stolen memory base address on gen2

 arch/x86/kernel/early-quirks.c         | 147 ++++++++++++++++++++++++++-------
 drivers/char/agp/intel-gtt.c           |  35 ++++----
 drivers/gpu/drm/i915/i915_drv.h        |   7 +-
 drivers/gpu/drm/i915/i915_gem.c        |  10 ++-
 drivers/gpu/drm/i915/i915_gem_gtt.c    |  32 +++++--
 drivers/gpu/drm/i915/i915_gem_stolen.c |  51 ++++++++----
 include/drm/i915_drm.h                 |  12 +++
 include/drm/intel-gtt.h                |   3 +-
 8 files changed, 223 insertions(+), 74 deletions(-)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 1/8] x86: Add vfunc for Intel graphics stolen memory base address

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

For gen2 devices we're going to need another way to determine the
stolen memory base address. Make that into a vfunc as well.

Also drop the bogus inline keyword from gen8_stolen_size().

Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: x86@kernel.org
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 arch/x86/kernel/early-quirks.c | 77 ++++++++++++++++++++++++++----------------
 1 file changed, 48 insertions(+), 29 deletions(-)

diff --git a/arch/x86/kernel/early-quirks.c b/arch/x86/kernel/early-quirks.c
index bc4a088..ca49966 100644
--- a/arch/x86/kernel/early-quirks.c
+++ b/arch/x86/kernel/early-quirks.c
@@ -228,7 +228,7 @@ static void __init intel_remapping_check(int num, int slot, int func)
  *
  * And yes, so far on current devices the base addr is always under 4G.
  */
-static u32 __init intel_stolen_base(int num, int slot, int func)
+static u32 __init intel_stolen_base(int num, int slot, int func, size_t size)
 {
 	u32 base;
 
@@ -313,7 +313,7 @@ static size_t __init gen6_stolen_size(int num, int slot, int func)
 	return gmch_ctrl << 25; /* 32 MB units */
 }
 
-static inline size_t gen8_stolen_size(int num, int slot, int func)
+static size_t gen8_stolen_size(int num, int slot, int func)
 {
 	u16 gmch_ctrl;
 
@@ -323,31 +323,50 @@ static inline size_t gen8_stolen_size(int num, int slot, int func)
 	return gmch_ctrl << 25; /* 32 MB units */
 }
 
-typedef size_t (*stolen_size_fn)(int num, int slot, int func);
+
+struct intel_stolen_funcs {
+	size_t (*size)(int num, int slot, int func);
+	u32 (*base)(int num, int slot, int func, size_t size);
+};
+
+static const struct intel_stolen_funcs gen3_stolen_funcs = {
+	.base = intel_stolen_base,
+	.size = gen3_stolen_size,
+};
+
+static const struct intel_stolen_funcs gen6_stolen_funcs = {
+	.base = intel_stolen_base,
+	.size = gen6_stolen_size,
+};
+
+static const struct intel_stolen_funcs gen8_stolen_funcs = {
+	.base = intel_stolen_base,
+	.size = gen8_stolen_size,
+};
 
 static struct pci_device_id intel_stolen_ids[] __initdata = {
-	INTEL_I915G_IDS(gen3_stolen_size),
-	INTEL_I915GM_IDS(gen3_stolen_size),
-	INTEL_I945G_IDS(gen3_stolen_size),
-	INTEL_I945GM_IDS(gen3_stolen_size),
-	INTEL_VLV_M_IDS(gen6_stolen_size),
-	INTEL_VLV_D_IDS(gen6_stolen_size),
-	INTEL_PINEVIEW_IDS(gen3_stolen_size),
-	INTEL_I965G_IDS(gen3_stolen_size),
-	INTEL_G33_IDS(gen3_stolen_size),
-	INTEL_I965GM_IDS(gen3_stolen_size),
-	INTEL_GM45_IDS(gen3_stolen_size),
-	INTEL_G45_IDS(gen3_stolen_size),
-	INTEL_IRONLAKE_D_IDS(gen3_stolen_size),
-	INTEL_IRONLAKE_M_IDS(gen3_stolen_size),
-	INTEL_SNB_D_IDS(gen6_stolen_size),
-	INTEL_SNB_M_IDS(gen6_stolen_size),
-	INTEL_IVB_M_IDS(gen6_stolen_size),
-	INTEL_IVB_D_IDS(gen6_stolen_size),
-	INTEL_HSW_D_IDS(gen6_stolen_size),
-	INTEL_HSW_M_IDS(gen6_stolen_size),
-	INTEL_BDW_M_IDS(gen8_stolen_size),
-	INTEL_BDW_D_IDS(gen8_stolen_size)
+	INTEL_I915G_IDS(&gen3_stolen_funcs),
+	INTEL_I915GM_IDS(&gen3_stolen_funcs),
+	INTEL_I945G_IDS(&gen3_stolen_funcs),
+	INTEL_I945GM_IDS(&gen3_stolen_funcs),
+	INTEL_VLV_M_IDS(&gen6_stolen_funcs),
+	INTEL_VLV_D_IDS(&gen6_stolen_funcs),
+	INTEL_PINEVIEW_IDS(&gen3_stolen_funcs),
+	INTEL_I965G_IDS(&gen3_stolen_funcs),
+	INTEL_G33_IDS(&gen3_stolen_funcs),
+	INTEL_I965GM_IDS(&gen3_stolen_funcs),
+	INTEL_GM45_IDS(&gen3_stolen_funcs),
+	INTEL_G45_IDS(&gen3_stolen_funcs),
+	INTEL_IRONLAKE_D_IDS(&gen3_stolen_funcs),
+	INTEL_IRONLAKE_M_IDS(&gen3_stolen_funcs),
+	INTEL_SNB_D_IDS(&gen6_stolen_funcs),
+	INTEL_SNB_M_IDS(&gen6_stolen_funcs),
+	INTEL_IVB_M_IDS(&gen6_stolen_funcs),
+	INTEL_IVB_D_IDS(&gen6_stolen_funcs),
+	INTEL_HSW_D_IDS(&gen6_stolen_funcs),
+	INTEL_HSW_M_IDS(&gen6_stolen_funcs),
+	INTEL_BDW_M_IDS(&gen8_stolen_funcs),
+	INTEL_BDW_D_IDS(&gen8_stolen_funcs)
 };
 
 static void __init intel_graphics_stolen(int num, int slot, int func)
@@ -364,10 +383,10 @@ static void __init intel_graphics_stolen(int num, int slot, int func)
 
 	for (i = 0; i < ARRAY_SIZE(intel_stolen_ids); i++) {
 		if (intel_stolen_ids[i].device == device) {
-			stolen_size_fn stolen_size =
-				(stolen_size_fn)intel_stolen_ids[i].driver_data;
-			size = stolen_size(num, slot, func);
-			start = intel_stolen_base(num, slot, func);
+			const struct intel_stolen_funcs *stolen_funcs =
+				(const struct intel_stolen_funcs *)intel_stolen_ids[i].driver_data;
+			size = stolen_funcs->size(num, slot, func);
+			start = stolen_funcs->base(num, slot, func, size);
 			if (size && start) {
 				/* Mark this space as reserved */
 				e820_add_region(start, size, E820_RESERVED);
-- 
1.8.3.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/8] x86: Add Intel graphics stolen memory quirk for gen2 platforms

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

There doesn't seem to an explicit stolen memory base register on gen2.
Some old comment in the i915 code suggests we should get it via
max_low_pfn_mapped, but that's clearly a bad idea on my MGM.

The e820 map in said machine looks like this:
[    0.000000] BIOS-e820: [mem 0x0000000000000000-0x000000000009f7ff] usable
[    0.000000] BIOS-e820: [mem 0x000000000009f800-0x000000000009ffff] reserved
[    0.000000] BIOS-e820: [mem 0x00000000000ce000-0x00000000000cffff] reserved
[    0.000000] BIOS-e820: [mem 0x00000000000dc000-0x00000000000fffff] reserved
[    0.000000] BIOS-e820: [mem 0x0000000000100000-0x000000001f6effff] usable
[    0.000000] BIOS-e820: [mem 0x000000001f6f0000-0x000000001f6f7fff] ACPI data
[    0.000000] BIOS-e820: [mem 0x000000001f6f8000-0x000000001f6fffff] ACPI NVS
[    0.000000] BIOS-e820: [mem 0x000000001f700000-0x000000001fffffff] reserved
[    0.000000] BIOS-e820: [mem 0x00000000fec10000-0x00000000fec1ffff] reserved
[    0.000000] BIOS-e820: [mem 0x00000000ffb00000-0x00000000ffbfffff] reserved
[    0.000000] BIOS-e820: [mem 0x00000000fff00000-0x00000000ffffffff] reserved

That makes max_low_pfn_mapped = 1f6f0000, so assuming our stolen memory
would start there would place it on top of some ACPI memory regions.
So not a good idea as already stated.

The 9MB region after the ACPI regions at 0x1f700000 however looks
promising given that the macine reports the stolen memory size to be
8MB. Looking at the PGTBL_CTL register, the GTT entries are at offset
0x1fee00000, and given that the GTT entries occupy 128KB, it looks like
the stolen memory could start at 0x1f700000 and the GTT entries would
occupy the last 128KB of the stolen memory. I have no idea about the
extra 1MB after the GTT entries.

I tested this on the machine in question, and so far I've not seen any
issues with the use the this memory region. Hopefully the same rules
hold for all gen2 machines...

Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: x86@kernel.org
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 arch/x86/kernel/early-quirks.c | 70 ++++++++++++++++++++++++++++++++++++++++++
 include/drm/i915_drm.h         | 12 ++++++++
 2 files changed, 82 insertions(+)

diff --git a/arch/x86/kernel/early-quirks.c b/arch/x86/kernel/early-quirks.c
index ca49966..6cd90b4 100644
--- a/arch/x86/kernel/early-quirks.c
+++ b/arch/x86/kernel/early-quirks.c
@@ -247,6 +247,62 @@ static u32 __init intel_stolen_base(int num, int slot, int func, size_t size)
 #define MB(x)	(KB (KB (x)))
 #define GB(x)	(MB (KB (x)))
 
+/*
+ * Gen2 stolen base is tricky. Try to deduce it from the
+ * page table base address.
+ */
+static u32 __init gen2_stolen_base(int num, int slot, int func, size_t size)
+{
+	void __iomem *regs;
+	u32 reg_addr;
+	u32 pgtbl_ctl;
+
+	reg_addr = read_pci_config(0, 2, 0, I810_MMADDR) & 0xfff80000;
+
+	regs = early_ioremap(reg_addr, KB(64));
+	if (!regs)
+		return 0;
+	pgtbl_ctl = readl(regs + I810_PGTBL_CTL);
+	early_iounmap(regs, KB(64));
+
+	/* GTT disabled? */
+	if (!(pgtbl_ctl & I810_PGTBL_ENABLED))
+		return 0;
+
+	pgtbl_ctl &= I810_PGTBL_ADDRESS_MASK;
+
+	/* Assume GTT entries occupy the last 128KB of stolen/local memory */
+	return pgtbl_ctl + KB(128) - size;
+}
+
+static size_t __init i830_stolen_size(int num, int slot, int func)
+{
+	size_t stolen_size;
+	u16 gmch_ctrl;
+
+	gmch_ctrl = read_pci_config_16(0, 0, 0, I830_GMCH_CTRL);
+
+	switch (gmch_ctrl & I830_GMCH_GMS_MASK) {
+	case I830_GMCH_GMS_STOLEN_512:
+		stolen_size = KB(512);
+		break;
+	case I830_GMCH_GMS_STOLEN_1024:
+		stolen_size = MB(1);
+		break;
+	case I830_GMCH_GMS_STOLEN_8192:
+		stolen_size = MB(8);
+		break;
+	case I830_GMCH_GMS_LOCAL:
+		/* local memory isn't part of the normal address space */
+		stolen_size = 0;
+		break;
+	default:
+		return 0;
+	}
+
+	return stolen_size;
+}
+
 static size_t __init gen3_stolen_size(int num, int slot, int func)
 {
 	size_t stolen_size;
@@ -329,6 +385,16 @@ struct intel_stolen_funcs {
 	u32 (*base)(int num, int slot, int func, size_t size);
 };
 
+static const struct intel_stolen_funcs i830_stolen_funcs = {
+	.base = gen2_stolen_base,
+	.size = i830_stolen_size,
+};
+
+static const struct intel_stolen_funcs i85x_stolen_funcs = {
+	.base = gen2_stolen_base,
+	.size = gen3_stolen_size,
+};
+
 static const struct intel_stolen_funcs gen3_stolen_funcs = {
 	.base = intel_stolen_base,
 	.size = gen3_stolen_size,
@@ -345,6 +411,10 @@ static const struct intel_stolen_funcs gen8_stolen_funcs = {
 };
 
 static struct pci_device_id intel_stolen_ids[] __initdata = {
+	INTEL_I830_IDS(&i830_stolen_funcs),
+	INTEL_I845G_IDS(&i830_stolen_funcs),
+	INTEL_I85X_IDS(&i85x_stolen_funcs),
+	INTEL_I865G_IDS(&i85x_stolen_funcs),
 	INTEL_I915G_IDS(&gen3_stolen_funcs),
 	INTEL_I915GM_IDS(&gen3_stolen_funcs),
 	INTEL_I945G_IDS(&gen3_stolen_funcs),
diff --git a/include/drm/i915_drm.h b/include/drm/i915_drm.h
index 97d5497..1526546 100644
--- a/include/drm/i915_drm.h
+++ b/include/drm/i915_drm.h
@@ -54,8 +54,16 @@ extern bool i915_gpu_turbo_disable(void);
 #define    BDW_GMCH_GMS_SHIFT   8
 #define    BDW_GMCH_GMS_MASK    0xff
 
+#define I810_MMADDR		0x14
+
 #define I830_GMCH_CTRL			0x52
 
+#define I830_GMCH_GMS_MASK		0x70
+#define I830_GMCH_GMS_LOCAL		0x10
+#define I830_GMCH_GMS_STOLEN_512	0x20
+#define I830_GMCH_GMS_STOLEN_1024	0x30
+#define I830_GMCH_GMS_STOLEN_8192	0x40
+
 #define I855_GMCH_GMS_MASK		0xF0
 #define I855_GMCH_GMS_STOLEN_0M		0x0
 #define I855_GMCH_GMS_STOLEN_1M		(0x1 << 4)
@@ -72,4 +80,8 @@ extern bool i915_gpu_turbo_disable(void);
 #define INTEL_GMCH_GMS_STOLEN_224M	(0xc << 4)
 #define INTEL_GMCH_GMS_STOLEN_352M	(0xd << 4)
 
+#define I810_PGTBL_CTL			0x2020
+#define    I810_PGTBL_ADDRESS_MASK	0xfffff000
+#define    I810_PGTBL_ENABLED		(1 << 0)
+
 #endif				/* _I915_DRM_H_ */
-- 
1.8.3.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 3/8] intel-gtt: Return whether we have local memory or not

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/char/agp/intel-gtt.c        | 16 ++++++++++------
 drivers/gpu/drm/i915/i915_gem_gtt.c |  3 ++-
 include/drm/intel-gtt.h             |  3 ++-
 3 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/drivers/char/agp/intel-gtt.c b/drivers/char/agp/intel-gtt.c
index 078968d..99ff699 100644
--- a/drivers/char/agp/intel-gtt.c
+++ b/drivers/char/agp/intel-gtt.c
@@ -68,6 +68,7 @@ static struct _intel_private {
 	u32 PGETBL_save;
 	u32 __iomem *gtt;		/* I915G */
 	bool clear_fake_agp; /* on first access via agp, fill with scratch */
+	bool has_local_memory;
 	int num_dcache_entries;
 	void __iomem *i9xx_flush_page;
 	char *i81x_gtt_table;
@@ -345,14 +346,15 @@ static const struct aper_size_info_fixed intel_fake_agp_sizes[] = {
 	{512, 131072, 7},
 };
 
-static unsigned int intel_gtt_stolen_size(void)
+static unsigned int intel_gtt_stolen_size(bool *has_local_memory)
 {
 	u16 gmch_ctrl;
 	u8 rdct;
-	int local = 0;
 	static const int ddt[4] = { 0, 16, 32, 64 };
 	unsigned int stolen_size = 0;
 
+	*has_local_memory = false;
+
 	if (INTEL_GTT_GEN == 1)
 		return 0; /* no stolen mem on i81x */
 
@@ -375,7 +377,7 @@ static unsigned int intel_gtt_stolen_size(void)
 			rdct = readb(intel_private.registers+I830_RDRAM_CHANNEL_TYPE);
 			stolen_size = (I830_RDRAM_ND(rdct) + 1) *
 					MB(ddt[I830_RDRAM_DDT(rdct)]);
-			local = 1;
+			*has_local_memory = true;
 			break;
 		default:
 			stolen_size = 0;
@@ -430,7 +432,7 @@ static unsigned int intel_gtt_stolen_size(void)
 
 	if (stolen_size > 0) {
 		dev_info(&intel_private.bridge_dev->dev, "detected %dK %s memory\n",
-		       stolen_size / KB(1), local ? "local" : "stolen");
+		       stolen_size / KB(1), *has_local_memory ? "local" : "stolen");
 	} else {
 		dev_info(&intel_private.bridge_dev->dev,
 		       "no pre-allocated video memory detected\n");
@@ -655,7 +657,7 @@ static int intel_gtt_init(void)
 	global_cache_flush();   /* FIXME: ? */
 #endif
 
-	intel_private.stolen_size = intel_gtt_stolen_size();
+	intel_private.stolen_size = intel_gtt_stolen_size(&intel_private.has_local_memory);
 
 	intel_private.needs_dmar = USE_PCI_DMA_API && INTEL_GTT_GEN > 2;
 
@@ -1423,12 +1425,14 @@ int intel_gmch_probe(struct pci_dev *bridge_pdev, struct pci_dev *gpu_pdev,
 EXPORT_SYMBOL(intel_gmch_probe);
 
 void intel_gtt_get(size_t *gtt_total, size_t *stolen_size,
-		   phys_addr_t *mappable_base, unsigned long *mappable_end)
+		   phys_addr_t *mappable_base, unsigned long *mappable_end,
+		   bool *has_local_memory)
 {
 	*gtt_total = intel_private.gtt_total_entries << PAGE_SHIFT;
 	*stolen_size = intel_private.stolen_size;
 	*mappable_base = intel_private.gma_bus_addr;
 	*mappable_end = intel_private.gtt_mappable_entries << PAGE_SHIFT;
+	*has_local_memory = intel_private.has_local_memory;
 }
 EXPORT_SYMBOL(intel_gtt_get);
 
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index a54eaab..70b148c 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -1426,6 +1426,7 @@ static int i915_gmch_probe(struct drm_device *dev,
 			   unsigned long *mappable_end)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
+	bool has_local_memory;
 	int ret;
 
 	ret = intel_gmch_probe(dev_priv->bridge_dev, dev_priv->dev->pdev, NULL);
@@ -1434,7 +1435,7 @@ static int i915_gmch_probe(struct drm_device *dev,
 		return -EIO;
 	}
 
-	intel_gtt_get(gtt_total, stolen, mappable_base, mappable_end);
+	intel_gtt_get(gtt_total, stolen, mappable_base, mappable_end, &has_local_memory);
 
 	dev_priv->gtt.do_idle_maps = needs_idle_maps(dev_priv->dev);
 	dev_priv->gtt.base.clear_range = i915_ggtt_clear_range;
diff --git a/include/drm/intel-gtt.h b/include/drm/intel-gtt.h
index b08bdad..827fe29 100644
--- a/include/drm/intel-gtt.h
+++ b/include/drm/intel-gtt.h
@@ -4,7 +4,8 @@
 #define	_DRM_INTEL_GTT_H
 
 void intel_gtt_get(size_t *gtt_total, size_t *stolen_size,
-		   phys_addr_t *mappable_base, unsigned long *mappable_end);
+		   phys_addr_t *mappable_base, unsigned long *mappable_end,
+		   bool *has_local_memory);
 
 int intel_gmch_probe(struct pci_dev *bridge_pdev, struct pci_dev *gpu_pdev,
 		     struct agp_bridge_data *bridge);
-- 
1.8.3.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 4/8] intel-gtt: Assume last 128KB of stolen contains the GTT entries on gen2

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

At least on my MGM the last 128KB of, what I assume is the stolen memory
region, contains the GTT entries. Let's assume that holds for all gen2
platforms.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/char/agp/intel-gtt.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/char/agp/intel-gtt.c b/drivers/char/agp/intel-gtt.c
index 99ff699..35788a2 100644
--- a/drivers/char/agp/intel-gtt.c
+++ b/drivers/char/agp/intel-gtt.c
@@ -430,6 +430,10 @@ static unsigned int intel_gtt_stolen_size(bool *has_local_memory)
 		}
 	}
 
+	/* Assume GTT entries occupy the last 128KB of stolen/local memory. */
+	if (stolen_size > 0 && INTEL_GTT_GEN == 2)
+		stolen_size -= KB(128);
+
 	if (stolen_size > 0) {
 		dev_info(&intel_private.bridge_dev->dev, "detected %dK %s memory\n",
 		       stolen_size / KB(1), *has_local_memory ? "local" : "stolen");
-- 
1.8.3.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 5/8] intel-gtt: Use i810_write_entry() on gen2 platforms

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

i810_write_entry() is identical to i830_write_entry(), except the the
use of the AGP_DCACHE_MEMORY flag to signal that the PTE should be
encoded with I810_PTE_LOCAL. We want that on i830 if we have local
memory. So simply swap all gen2 machines over to i810_write_entry()

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/char/agp/intel-gtt.c | 15 ++-------------
 1 file changed, 2 insertions(+), 13 deletions(-)

diff --git a/drivers/char/agp/intel-gtt.c b/drivers/char/agp/intel-gtt.c
index 35788a2..35a667d 100644
--- a/drivers/char/agp/intel-gtt.c
+++ b/drivers/char/agp/intel-gtt.c
@@ -743,17 +743,6 @@ static void i830_chipset_flush(void)
 	}
 }
 
-static void i830_write_entry(dma_addr_t addr, unsigned int entry,
-			     unsigned int flags)
-{
-	u32 pte_flags = I810_PTE_VALID;
-
-	if (flags ==  AGP_USER_CACHED_MEMORY)
-		pte_flags |= I830_PTE_SYSTEM_CACHED;
-
-	writel(addr | pte_flags, intel_private.gtt + entry);
-}
-
 bool intel_enable_gtt(void)
 {
 	u8 __iomem *reg;
@@ -1196,7 +1185,7 @@ static const struct intel_gtt_driver i8xx_gtt_driver = {
 	.has_pgtbl_enable = 1,
 	.setup = i830_setup,
 	.cleanup = i830_cleanup,
-	.write_entry = i830_write_entry,
+	.write_entry = i810_write_entry,
 	.dma_mask_size = 32,
 	.check_flags = i830_check_flags,
 	.chipset_flush = i830_chipset_flush,
@@ -1207,7 +1196,7 @@ static const struct intel_gtt_driver i915_gtt_driver = {
 	.setup = i9xx_setup,
 	.cleanup = i9xx_cleanup,
 	/* i945 is the last gpu to need phys mem (for overlay and cursors). */
-	.write_entry = i830_write_entry,
+	.write_entry = i810_write_entry,
 	.dma_mask_size = 32,
 	.check_flags = i830_check_flags,
 	.chipset_flush = i9xx_chipset_flush,
-- 
1.8.3.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 6/8] drm/i915: Add I915_CACHE_LOCAL to indicate local memory

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

When using local memory, we need to encode the PTEs correctly.
Since cache_mode is the only thing that gets passed down there,
add a new cache_level that's only used for objects allocated from
local memory.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h     |  1 +
 drivers/gpu/drm/i915/i915_gem.c     | 10 +++++++++-
 drivers/gpu/drm/i915/i915_gem_gtt.c | 16 +++++++++++++---
 3 files changed, 23 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 20a9811..9ee725f 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -515,6 +515,7 @@ enum i915_cache_level {
 			      large Last-Level-Cache. LLC is coherent with
 			      the CPU, but L3 is only visible to the GPU. */
 	I915_CACHE_WT, /* hsw:gt3e WriteThrough for scanouts */
+	I915_CACHE_LOCAL, /* local memory */
 };
 
 typedef uint32_t gen6_gtt_pte_t;
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 1bd8953..b08d5d3 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1392,7 +1392,8 @@ int i915_gem_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
 	trace_i915_gem_object_fault(obj, page_offset, true, write);
 
 	/* Access to snoopable pages through the GTT is incoherent. */
-	if (obj->cache_level != I915_CACHE_NONE && !HAS_LLC(dev)) {
+	if (obj->cache_level != I915_CACHE_NONE &&
+	    obj->cache_level != I915_CACHE_LOCAL && !HAS_LLC(dev)) {
 		ret = -EINVAL;
 		goto unlock;
 	}
@@ -3478,6 +3479,13 @@ int i915_gem_object_set_cache_level(struct drm_i915_gem_object *obj,
 	if (obj->cache_level == cache_level)
 		return 0;
 
+	if (obj->cache_level == I915_CACHE_LOCAL) {
+		if (cache_level != I915_CACHE_NONE)
+			return -EINVAL;
+
+		return 0;
+	}
+
 	if (obj->pin_count) {
 		DRM_DEBUG("can not change the cache level of pinned objects\n");
 		return -EBUSY;
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 70b148c..0eb6203 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -1022,11 +1022,21 @@ static void i915_ggtt_insert_entries(struct i915_address_space *vm,
 				     unsigned int pg_start,
 				     enum i915_cache_level cache_level)
 {
-	unsigned int flags = (cache_level == I915_CACHE_NONE) ?
-		AGP_USER_MEMORY : AGP_USER_CACHED_MEMORY;
+	unsigned int flags;
 
-	intel_gtt_insert_sg_entries(st, pg_start, flags);
+	switch (cache_level) {
+	case I915_CACHE_NONE:
+		flags = AGP_USER_MEMORY;
+		break;
+	case I915_CACHE_LOCAL:
+		flags = AGP_DCACHE_MEMORY;
+		break;
+	default:
+		flags = AGP_USER_CACHED_MEMORY;
+		break;
+	}
 
+	intel_gtt_insert_sg_entries(st, pg_start, flags);
 }
 
 static void i915_ggtt_clear_range(struct i915_address_space *vm,
-- 
1.8.3.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 7/8] drm/i915: Keep track if we have local memory

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

intel-gtt will tell us if we have local memory as opposed to stolen
memory. Make a note of that in dev_priv->gtt.has_local_memory.

Local memory starts at offset 0 from the GPU's point of view, so we
need to add a small exception to consider stolen_base==0 as valid
when local memory is present.

The other part of the story is encoding the PTEs correctly. To make
that happen set the cache_level to I915_CACHE_LOCAL for all
stolen objects when local memory is present.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h        |  6 ++++--
 drivers/gpu/drm/i915/i915_gem_gtt.c    | 17 +++++++++++------
 drivers/gpu/drm/i915/i915_gem_stolen.c | 15 +++++++++++----
 3 files changed, 26 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 9ee725f..2d9a1b3 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -589,13 +589,15 @@ struct i915_gtt {
 	void __iomem *gsm;
 
 	bool do_idle_maps;
+	bool has_local_memory;
 
 	int mtrr;
 
 	/* global gtt ops */
 	int (*gtt_probe)(struct drm_device *dev, size_t *gtt_total,
-			  size_t *stolen, phys_addr_t *mappable_base,
-			  unsigned long *mappable_end);
+			 size_t *stolen, phys_addr_t *mappable_base,
+			 unsigned long *mappable_end,
+			 bool *has_local_memory);
 };
 #define gtt_total_entries(gtt) ((gtt).base.total >> PAGE_SHIFT)
 
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 0eb6203..c154d80 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -1348,7 +1348,8 @@ static int gen8_gmch_probe(struct drm_device *dev,
 			   size_t *gtt_total,
 			   size_t *stolen,
 			   phys_addr_t *mappable_base,
-			   unsigned long *mappable_end)
+			   unsigned long *mappable_end,
+			   bool *has_local_memory)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	unsigned int gtt_size;
@@ -1358,6 +1359,7 @@ static int gen8_gmch_probe(struct drm_device *dev,
 	/* TODO: We're not aware of mappable constraints on gen8 yet */
 	*mappable_base = pci_resource_start(dev->pdev, 2);
 	*mappable_end = pci_resource_len(dev->pdev, 2);
+	*has_local_memory = false;
 
 	if (!pci_set_dma_mask(dev->pdev, DMA_BIT_MASK(39)))
 		pci_set_consistent_dma_mask(dev->pdev, DMA_BIT_MASK(39));
@@ -1383,7 +1385,8 @@ static int gen6_gmch_probe(struct drm_device *dev,
 			   size_t *gtt_total,
 			   size_t *stolen,
 			   phys_addr_t *mappable_base,
-			   unsigned long *mappable_end)
+			   unsigned long *mappable_end,
+			   bool *has_local_memory)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	unsigned int gtt_size;
@@ -1392,6 +1395,7 @@ static int gen6_gmch_probe(struct drm_device *dev,
 
 	*mappable_base = pci_resource_start(dev->pdev, 2);
 	*mappable_end = pci_resource_len(dev->pdev, 2);
+	*has_local_memory = false;
 
 	/* 64/512MB is the current min/max we actually know of, but this is just
 	 * a coarse sanity check.
@@ -1433,10 +1437,10 @@ static int i915_gmch_probe(struct drm_device *dev,
 			   size_t *gtt_total,
 			   size_t *stolen,
 			   phys_addr_t *mappable_base,
-			   unsigned long *mappable_end)
+			   unsigned long *mappable_end,
+			   bool *has_local_memory)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	bool has_local_memory;
 	int ret;
 
 	ret = intel_gmch_probe(dev_priv->bridge_dev, dev_priv->dev->pdev, NULL);
@@ -1445,7 +1449,7 @@ static int i915_gmch_probe(struct drm_device *dev,
 		return -EIO;
 	}
 
-	intel_gtt_get(gtt_total, stolen, mappable_base, mappable_end, &has_local_memory);
+	intel_gtt_get(gtt_total, stolen, mappable_base, mappable_end, has_local_memory);
 
 	dev_priv->gtt.do_idle_maps = needs_idle_maps(dev_priv->dev);
 	dev_priv->gtt.base.clear_range = i915_ggtt_clear_range;
@@ -1487,7 +1491,8 @@ int i915_gem_gtt_init(struct drm_device *dev)
 	}
 
 	ret = gtt->gtt_probe(dev, &gtt->base.total, &gtt->stolen_size,
-			     &gtt->mappable_base, &gtt->mappable_end);
+			     &gtt->mappable_base, &gtt->mappable_end,
+			     &gtt->has_local_memory);
 	if (ret)
 		return ret;
 
diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
index d284d89..39e6404 100644
--- a/drivers/gpu/drm/i915/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
@@ -205,11 +205,14 @@ int i915_gem_init_stolen(struct drm_device *dev)
 		return 0;
 
 	dev_priv->mm.stolen_base = i915_stolen_to_physical(dev);
-	if (dev_priv->mm.stolen_base == 0)
+	if (!dev_priv->gtt.has_local_memory &&
+	    dev_priv->mm.stolen_base == 0)
 		return 0;
 
-	DRM_DEBUG_KMS("found %zd bytes of stolen memory at %08lx\n",
-		      dev_priv->gtt.stolen_size, dev_priv->mm.stolen_base);
+	DRM_DEBUG_KMS("found %zd bytes of %s memory at %08lx\n",
+		      dev_priv->gtt.stolen_size,
+		      dev_priv->gtt.has_local_memory ? "local" : "stolen",
+		      dev_priv->mm.stolen_base);
 
 	if (IS_VALLEYVIEW(dev))
 		bios_reserved = 1024*1024; /* top 1M on VLV/BYT */
@@ -281,6 +284,7 @@ static struct drm_i915_gem_object *
 _i915_gem_object_create_stolen(struct drm_device *dev,
 			       struct drm_mm_node *stolen)
 {
+	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct drm_i915_gem_object *obj;
 
 	obj = i915_gem_object_alloc(dev);
@@ -300,7 +304,10 @@ _i915_gem_object_create_stolen(struct drm_device *dev,
 	obj->stolen = stolen;
 
 	obj->base.read_domains = I915_GEM_DOMAIN_CPU | I915_GEM_DOMAIN_GTT;
-	obj->cache_level = HAS_LLC(dev) ? I915_CACHE_LLC : I915_CACHE_NONE;
+	if (dev_priv->gtt.has_local_memory)
+		obj->cache_level = I915_CACHE_LOCAL;
+	else
+		obj->cache_level = HAS_LLC(dev) ? I915_CACHE_LLC : I915_CACHE_NONE;
 
 	return obj;
 
-- 
1.8.3.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 8/8] drm/i915: Determine the stolen memory base address on gen2

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

There doesn't seem to an explicit stolen memory base register on gen2.
Some old comment in the code suggests we should get it via
max_low_pfn_mapped, but that's clearly a bad idea on my MGM.

The e820 map in said machine looks like this:
[    0.000000] BIOS-e820: [mem 0x0000000000000000-0x000000000009f7ff] usable
[    0.000000] BIOS-e820: [mem 0x000000000009f800-0x000000000009ffff] reserved
[    0.000000] BIOS-e820: [mem 0x00000000000ce000-0x00000000000cffff] reserved
[    0.000000] BIOS-e820: [mem 0x00000000000dc000-0x00000000000fffff] reserved
[    0.000000] BIOS-e820: [mem 0x0000000000100000-0x000000001f6effff] usable
[    0.000000] BIOS-e820: [mem 0x000000001f6f0000-0x000000001f6f7fff] ACPI data
[    0.000000] BIOS-e820: [mem 0x000000001f6f8000-0x000000001f6fffff] ACPI NVS
[    0.000000] BIOS-e820: [mem 0x000000001f700000-0x000000001fffffff] reserved
[    0.000000] BIOS-e820: [mem 0x00000000fec10000-0x00000000fec1ffff] reserved
[    0.000000] BIOS-e820: [mem 0x00000000ffb00000-0x00000000ffbfffff] reserved
[    0.000000] BIOS-e820: [mem 0x00000000fff00000-0x00000000ffffffff] reserved

That makes max_low_pfn_mapped = 1f6f0000, so assuming our stolen memory
would start there would place it on top of some ACPI memory regions.
So not a good idea as already stated.

The 9MB region after the ACPI regions at 0x1f700000 however looks
promising given that the macine reports the stolen memory size to be
8MB. Looking at the PGTBL_CTL register, the GTT entries are at offset
0x1fee00000, and given that the GTT entries occupy 128KB, it looks like
the stolen memory could start at 0x1f700000 and the GTT entries would
occupy the last 128KB of the stolen memory. I have no idea about the
extra 1MB after the GTT entries.

I tested this on the machine in question, and so far I've not seen any
issues. Hopefully the same rules hold for all gen2 machines...

On i830 w/ local memory, stolen memory is replaced by the local memory.
In this case the page table is stored in local memory. Since the local
memory starts at offset 0, we can add an extra sanity check to make sure
the page table is really stored at the end of local memory.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_gem_stolen.c | 36 ++++++++++++++++++++++++----------
 1 file changed, 26 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
index 39e6404..407a9fa 100644
--- a/drivers/gpu/drm/i915/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
@@ -51,13 +51,10 @@ static unsigned long i915_stolen_to_physical(struct drm_device *dev)
 	/* Almost universally we can find the Graphics Base of Stolen Memory
 	 * at offset 0x5c in the igfx configuration space. On a few (desktop)
 	 * machines this is also mirrored in the bridge device at different
-	 * locations, or in the MCHBAR. On gen2, the layout is again slightly
-	 * different with the Graphics Segment immediately following Top of
-	 * Memory (or Top of Usable DRAM). Note it appears that TOUD is only
-	 * reported by 865g, so we just use the top of memory as determined
-	 * by the e820 probe.
+	 * locations, or in the MCHBAR.
 	 *
-	 * XXX However gen2 requires an unavailable symbol.
+	 * Gen2 stolen base is tricky. Try to deduce it from the
+	 * page table base address.
 	 */
 	base = 0;
 	if (INTEL_INFO(dev)->gen >= 3) {
@@ -65,10 +62,29 @@ static unsigned long i915_stolen_to_physical(struct drm_device *dev)
 		pci_read_config_dword(dev->pdev, 0x5c, &base);
 		base &= ~((1<<20) - 1);
 	} else { /* GEN2 */
-#if 0
-		/* Stolen is immediately above Top of Memory */
-		base = max_low_pfn_mapped << PAGE_SHIFT;
-#endif
+		u32 pgtbl_ctl = I915_READ(I810_PGTBL_CTL);
+
+		/* GTT disabled? */
+		if (!(pgtbl_ctl & I810_PGTBL_ENABLED)) {
+			dev_priv->gtt.has_local_memory = false;
+			return 0;
+		}
+
+		pgtbl_ctl &= I810_PGTBL_ADDRESS_MASK;
+
+		/*
+		 * Assume GTT entries occupy the last 128KB of stolen/local memory.
+		 * The 128KB should already be deducted from stolen_size by this
+		 * time.
+		 */
+		base = pgtbl_ctl - dev_priv->gtt.stolen_size;
+
+		if (WARN(dev_priv->gtt.has_local_memory && base != 0,
+			 "IGD page table address (0x%x) vs. local memory size (%zu) mismatch\n",
+			 pgtbl_ctl, dev_priv->gtt.stolen_size)) {
+			dev_priv->gtt.has_local_memory = false;
+			return 0;
+		}
 	}
 
 	if (base == 0)
-- 
1.8.3.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 8/8] drm/i915: Determine the stolen memory base address on gen2

[Chris Wilson]

On Thu, Nov 28, 2013 at 05:15:10PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> There doesn't seem to an explicit stolen memory base register on gen2.
> Some old comment in the code suggests we should get it via
> max_low_pfn_mapped, but that's clearly a bad idea on my MGM.
> 
> The e820 map in said machine looks like this:
> [    0.000000] BIOS-e820: [mem 0x0000000000000000-0x000000000009f7ff] usable
> [    0.000000] BIOS-e820: [mem 0x000000000009f800-0x000000000009ffff] reserved
> [    0.000000] BIOS-e820: [mem 0x00000000000ce000-0x00000000000cffff] reserved
> [    0.000000] BIOS-e820: [mem 0x00000000000dc000-0x00000000000fffff] reserved
> [    0.000000] BIOS-e820: [mem 0x0000000000100000-0x000000001f6effff] usable
> [    0.000000] BIOS-e820: [mem 0x000000001f6f0000-0x000000001f6f7fff] ACPI data
> [    0.000000] BIOS-e820: [mem 0x000000001f6f8000-0x000000001f6fffff] ACPI NVS
> [    0.000000] BIOS-e820: [mem 0x000000001f700000-0x000000001fffffff] reserved
> [    0.000000] BIOS-e820: [mem 0x00000000fec10000-0x00000000fec1ffff] reserved
> [    0.000000] BIOS-e820: [mem 0x00000000ffb00000-0x00000000ffbfffff] reserved
> [    0.000000] BIOS-e820: [mem 0x00000000fff00000-0x00000000ffffffff] reserved
> 
> That makes max_low_pfn_mapped = 1f6f0000, so assuming our stolen memory
> would start there would place it on top of some ACPI memory regions.
> So not a good idea as already stated.
> 
> The 9MB region after the ACPI regions at 0x1f700000 however looks
> promising given that the macine reports the stolen memory size to be
> 8MB. Looking at the PGTBL_CTL register, the GTT entries are at offset
> 0x1fee00000, and given that the GTT entries occupy 128KB, it looks like
> the stolen memory could start at 0x1f700000 and the GTT entries would
> occupy the last 128KB of the stolen memory. I have no idea about the
> extra 1MB after the GTT entries.

The base address for stolen memory is explicitly Top of Memory on gen2,
at least in 830/845 cspecs that I found (and recall). But this idea is
interesting...
-Chris
-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 8/8] drm/i915: Determine the stolen memory base address on gen2

[Ville Syrjälä]

On Thu, Nov 28, 2013 at 04:32:14PM +0000, Chris Wilson wrote:
> On Thu, Nov 28, 2013 at 05:15:10PM +0200, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > There doesn't seem to an explicit stolen memory base register on gen2.
> > Some old comment in the code suggests we should get it via
> > max_low_pfn_mapped, but that's clearly a bad idea on my MGM.
> > 
> > The e820 map in said machine looks like this:
> > [    0.000000] BIOS-e820: [mem 0x0000000000000000-0x000000000009f7ff] usable
> > [    0.000000] BIOS-e820: [mem 0x000000000009f800-0x000000000009ffff] reserved
> > [    0.000000] BIOS-e820: [mem 0x00000000000ce000-0x00000000000cffff] reserved
> > [    0.000000] BIOS-e820: [mem 0x00000000000dc000-0x00000000000fffff] reserved
> > [    0.000000] BIOS-e820: [mem 0x0000000000100000-0x000000001f6effff] usable
> > [    0.000000] BIOS-e820: [mem 0x000000001f6f0000-0x000000001f6f7fff] ACPI data
> > [    0.000000] BIOS-e820: [mem 0x000000001f6f8000-0x000000001f6fffff] ACPI NVS
> > [    0.000000] BIOS-e820: [mem 0x000000001f700000-0x000000001fffffff] reserved
> > [    0.000000] BIOS-e820: [mem 0x00000000fec10000-0x00000000fec1ffff] reserved
> > [    0.000000] BIOS-e820: [mem 0x00000000ffb00000-0x00000000ffbfffff] reserved
> > [    0.000000] BIOS-e820: [mem 0x00000000fff00000-0x00000000ffffffff] reserved
> > 
> > That makes max_low_pfn_mapped = 1f6f0000, so assuming our stolen memory
> > would start there would place it on top of some ACPI memory regions.
> > So not a good idea as already stated.
> > 
> > The 9MB region after the ACPI regions at 0x1f700000 however looks
> > promising given that the macine reports the stolen memory size to be
> > 8MB. Looking at the PGTBL_CTL register, the GTT entries are at offset
> > 0x1fee00000, and given that the GTT entries occupy 128KB, it looks like
> > the stolen memory could start at 0x1f700000 and the GTT entries would
> > occupy the last 128KB of the stolen memory. I have no idea about the
> > extra 1MB after the GTT entries.
> 
> The base address for stolen memory is explicitly Top of Memory on gen2,
> at least in 830/845 cspecs that I found (and recall). But this idea is
> interesting...

I have a hard time beliveing people would be nuts enough to place
the GTT entries somewhere the middle of the stolen memory area rather
than at the end. But I suppose I could be wrong.

There's also one additional reason why I suspect the last 1MB (or .5MB
on Daniel's machine) must be something else; it's covered by a UC MTRR,
whereas the presumed stolen memory range is not.

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH 2/8] x86: Add Intel graphics stolen memory quirk for gen2 platforms

[Ingo Molnar]

* ville.syrjala@linux.intel.com <ville.syrjala@linux.intel.com> wrote:

> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> There doesn't seem to an explicit stolen memory base register on gen2.
> Some old comment in the i915 code suggests we should get it via
> max_low_pfn_mapped, but that's clearly a bad idea on my MGM.
> 
> The e820 map in said machine looks like this:
> [    0.000000] BIOS-e820: [mem 0x0000000000000000-0x000000000009f7ff] usable
> [    0.000000] BIOS-e820: [mem 0x000000000009f800-0x000000000009ffff] reserved
> [    0.000000] BIOS-e820: [mem 0x00000000000ce000-0x00000000000cffff] reserved
> [    0.000000] BIOS-e820: [mem 0x00000000000dc000-0x00000000000fffff] reserved
> [    0.000000] BIOS-e820: [mem 0x0000000000100000-0x000000001f6effff] usable
> [    0.000000] BIOS-e820: [mem 0x000000001f6f0000-0x000000001f6f7fff] ACPI data
> [    0.000000] BIOS-e820: [mem 0x000000001f6f8000-0x000000001f6fffff] ACPI NVS
> [    0.000000] BIOS-e820: [mem 0x000000001f700000-0x000000001fffffff] reserved
> [    0.000000] BIOS-e820: [mem 0x00000000fec10000-0x00000000fec1ffff] reserved
> [    0.000000] BIOS-e820: [mem 0x00000000ffb00000-0x00000000ffbfffff] reserved
> [    0.000000] BIOS-e820: [mem 0x00000000fff00000-0x00000000ffffffff] reserved
> 
> That makes max_low_pfn_mapped = 1f6f0000, so assuming our stolen memory
> would start there would place it on top of some ACPI memory regions.
> So not a good idea as already stated.
> 
> The 9MB region after the ACPI regions at 0x1f700000 however looks
> promising given that the macine reports the stolen memory size to be
> 8MB. Looking at the PGTBL_CTL register, the GTT entries are at offset
> 0x1fee00000, and given that the GTT entries occupy 128KB, it looks like
> the stolen memory could start at 0x1f700000 and the GTT entries would
> occupy the last 128KB of the stolen memory. I have no idea about the
> extra 1MB after the GTT entries.
> 
> I tested this on the machine in question, and so far I've not seen any
> issues with the use the this memory region. Hopefully the same rules
> hold for all gen2 machines...
> 
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> Cc: x86@kernel.org
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  arch/x86/kernel/early-quirks.c | 70 ++++++++++++++++++++++++++++++++++++++++++
>  include/drm/i915_drm.h         | 12 ++++++++
>  2 files changed, 82 insertions(+)
> 
> diff --git a/arch/x86/kernel/early-quirks.c b/arch/x86/kernel/early-quirks.c
> index ca49966..6cd90b4 100644
> --- a/arch/x86/kernel/early-quirks.c
> +++ b/arch/x86/kernel/early-quirks.c
> @@ -247,6 +247,62 @@ static u32 __init intel_stolen_base(int num, int slot, int func, size_t size)
>  #define MB(x)	(KB (KB (x)))
>  #define GB(x)	(MB (KB (x)))
>  
> +/*
> + * Gen2 stolen base is tricky. Try to deduce it from the
> + * page table base address.
> + */
> +static u32 __init gen2_stolen_base(int num, int slot, int func, size_t size)
> +{
> +	void __iomem *regs;
> +	u32 reg_addr;
> +	u32 pgtbl_ctl;
> +
> +	reg_addr = read_pci_config(0, 2, 0, I810_MMADDR) & 0xfff80000;

Why is the mmaddr rounded to 512K? Do the lower 19 bits hold some 
other piece of information, or are they always zero?

> +	regs = early_ioremap(reg_addr, KB(64));
> +	if (!regs)
> +		return 0;

Emitting a warning here might be useful, if anyone runs into this.

> +	pgtbl_ctl = readl(regs + I810_PGTBL_CTL);
> +	early_iounmap(regs, KB(64));
> +
> +	/* GTT disabled? */
> +	if (!(pgtbl_ctl & I810_PGTBL_ENABLED))
> +		return 0;
> +
> +	pgtbl_ctl &= I810_PGTBL_ADDRESS_MASK;
> +
> +	/* Assume GTT entries occupy the last 128KB of stolen/local memory */
> +	return pgtbl_ctl + KB(128) - size;

Btw., I love the cleanliness of the KB() / MB() / GB() macros, it 
ought to move from agp.h to include/linux/kernel.h or so.


> +static size_t __init i830_stolen_size(int num, int slot, int func)
> +{
> +	size_t stolen_size;
> +	u16 gmch_ctrl;
> +
> +	gmch_ctrl = read_pci_config_16(0, 0, 0, I830_GMCH_CTRL);
> +
> +	switch (gmch_ctrl & I830_GMCH_GMS_MASK) {
> +	case I830_GMCH_GMS_STOLEN_512:
> +		stolen_size = KB(512);
> +		break;
> +	case I830_GMCH_GMS_STOLEN_1024:
> +		stolen_size = MB(1);
> +		break;
> +	case I830_GMCH_GMS_STOLEN_8192:
> +		stolen_size = MB(8);
> +		break;
> +	case I830_GMCH_GMS_LOCAL:
> +		/* local memory isn't part of the normal address space */
> +		stolen_size = 0;
> +		break;
> +	default:
> +		return 0;
> +	}
> +
> +	return stolen_size;

Nit: looks like the labels 'I830_GMCH_GMS_LOCAL' and 'default' have 
the same effect so they could be merged.

> diff --git a/include/drm/i915_drm.h b/include/drm/i915_drm.h
> index 97d5497..1526546 100644
> --- a/include/drm/i915_drm.h
> +++ b/include/drm/i915_drm.h
> @@ -54,8 +54,16 @@ extern bool i915_gpu_turbo_disable(void);
>  #define    BDW_GMCH_GMS_SHIFT   8
>  #define    BDW_GMCH_GMS_MASK    0xff
>  
> +#define I810_MMADDR		0x14
> +
>  #define I830_GMCH_CTRL			0x52

Nit: the I810_MMADDR define appears to be out of alignment with the 
surrounding constants, is that intentional?

Thanks,

	Ingo