Cleanup of early quirks

Cleanup of early quirks

[Joonas Lahtinen]

Preparatory series to de-duplicate the stolen memory calculation code
which currently resides both in early-quirks.c and i915_gem_stolen.c
files.

Open question is how to best deliver the information from the early
quirks code to the i915 driver? Adding a new E820 memory type is one
obvious option, and having a dedicated i915_early.o built-in to
receive and relay the information when i915 is built as module or
built-in.

Regards, Joonas
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 1/2] drm/i915: Canonicalize stolen memory calculations

[Joonas Lahtinen]

Move the better constructs/comments from i915_gem_stolen.c to
early-quirks.c and increase readability in preparation of only
having one set of functions.

- intel_stolen_base -> gen3_stolen_base
- use phys_addr_t instead of u32 for address for future proofing

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 arch/x86/kernel/early-quirks.c         | 234 +++++++++++++++------------------
 drivers/gpu/drm/i915/i915_gem_stolen.c |   4 +-
 include/drm/i915_drm.h                 |   3 +
 3 files changed, 109 insertions(+), 132 deletions(-)

diff --git a/arch/x86/kernel/early-quirks.c b/arch/x86/kernel/early-quirks.c
index bca14c8..f169475 100644
--- a/arch/x86/kernel/early-quirks.c
+++ b/arch/x86/kernel/early-quirks.c
@@ -223,36 +223,19 @@ static void __init intel_remapping_check(int num, int slot, int func)
  * despite the efforts of the "RAM buffer" approach, which simply rounds
  * memory boundaries up to 64M to try to catch space that may decode
  * as RAM and so is not suitable for MMIO.
- *
- * 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, size_t stolen_size)
-{
-	u32 base;
-
-	/*
-	 * For the PCI IDs in this quirk, the stolen base is always
-	 * in 0x5c, aka the BDSM register (yes that's really what
-	 * it's called).
-	 */
-	base = read_pci_config(num, slot, func, 0x5c);
-	base &= ~((1<<20) - 1);
-
-	return base;
-}
 
 #define KB(x)	((x) * 1024UL)
 #define MB(x)	(KB (KB (x)))
-#define GB(x)	(MB (KB (x)))
 
 static size_t __init i830_tseg_size(void)
 {
-	u8 tmp = read_pci_config_byte(0, 0, 0, I830_ESMRAMC);
+	u8 esmramc = read_pci_config_byte(0, 0, 0, I830_ESMRAMC);
 
-	if (!(tmp & TSEG_ENABLE))
+	if (!(esmramc & TSEG_ENABLE))
 		return 0;
 
-	if (tmp & I830_TSEG_SIZE_1M)
+	if (esmramc & I830_TSEG_SIZE_1M)
 		return MB(1);
 	else
 		return KB(512);
@@ -260,27 +243,26 @@ static size_t __init i830_tseg_size(void)
 
 static size_t __init i845_tseg_size(void)
 {
-	u8 tmp = read_pci_config_byte(0, 0, 0, I845_ESMRAMC);
+	u8 esmramc = read_pci_config_byte(0, 0, 0, I845_ESMRAMC);
+	u8 tseg_size = esmramc & I845_TSEG_SIZE_MASK;
 
-	if (!(tmp & TSEG_ENABLE))
+	if (!(esmramc & TSEG_ENABLE))
 		return 0;
 
-	switch (tmp & I845_TSEG_SIZE_MASK) {
-	case I845_TSEG_SIZE_512K:
-		return KB(512);
-	case I845_TSEG_SIZE_1M:
-		return MB(1);
+	switch (tseg_size) {
+	case I845_TSEG_SIZE_512K:	return KB(512);
+	case I845_TSEG_SIZE_1M:		return MB(1);
 	default:
-		WARN_ON(1);
-		return 0;
+		WARN(1, "Unknown register value!\n");
 	}
+	return 0;
 }
 
 static size_t __init i85x_tseg_size(void)
 {
-	u8 tmp = read_pci_config_byte(0, 0, 0, I85X_ESMRAMC);
+	u8 esmramc = read_pci_config_byte(0, 0, 0, I85X_ESMRAMC);
 
-	if (!(tmp & TSEG_ENABLE))
+	if (!(esmramc & TSEG_ENABLE))
 		return 0;
 
 	return MB(1);
@@ -300,174 +282,166 @@ static size_t __init i85x_mem_size(void)
  * On 830/845/85x the stolen memory base isn't available in any
  * register. We need to calculate it as TOM-TSEG_SIZE-stolen_size.
  */
-static u32 __init i830_stolen_base(int num, int slot, int func, size_t stolen_size)
+static phys_addr_t __init i830_stolen_base(int num, int slot, int func,
+					   size_t stolen_size)
 {
-	return i830_mem_size() - i830_tseg_size() - stolen_size;
+	return (phys_addr_t)i830_mem_size() - i830_tseg_size() - stolen_size;
 }
 
-static u32 __init i845_stolen_base(int num, int slot, int func, size_t stolen_size)
+static phys_addr_t __init i845_stolen_base(int num, int slot, int func,
+					   size_t stolen_size)
 {
-	return i830_mem_size() - i845_tseg_size() - stolen_size;
+	return (phys_addr_t)i830_mem_size() - i845_tseg_size() - stolen_size;
 }
 
-static u32 __init i85x_stolen_base(int num, int slot, int func, size_t stolen_size)
+static phys_addr_t __init i85x_stolen_base(int num, int slot, int func,
+					   size_t stolen_size)
 {
-	return i85x_mem_size() - i85x_tseg_size() - stolen_size;
+	return (phys_addr_t)i85x_mem_size() - i85x_tseg_size() - stolen_size;
 }
 
-static u32 __init i865_stolen_base(int num, int slot, int func, size_t stolen_size)
+static phys_addr_t __init i865_stolen_base(int num, int slot, int func,
+					   size_t stolen_size)
 {
+	u16 toud;
+
 	/*
 	 * FIXME is the graphics stolen memory region
 	 * always at TOUD? Ie. is it always the last
 	 * one to be allocated by the BIOS?
 	 */
-	return read_pci_config_16(0, 0, 0, I865_TOUD) << 16;
+	toud = read_pci_config_16(0, 0, 0, I865_TOUD);
+
+	return (phys_addr_t)toud << 16;
+}
+
+static phys_addr_t __init gen3_stolen_base(int num, int slot, int func,
+					   size_t stolen_size)
+{
+	u32 bsm;
+
+	/* Almost universally we can find the Graphics Base of Stolen Memory
+	 * at register BSM (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.
+	 */
+	bsm = read_pci_config(num, slot, func, INTEL_BSM);
+
+	return (phys_addr_t)bsm & INTEL_BSM_MASK;
 }
 
 static size_t __init i830_stolen_size(int num, int slot, int func)
 {
-	size_t stolen_size;
 	u16 gmch_ctrl;
+	u16 gms;
 
 	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;
+	gms = gmch_ctrl & I830_GMCH_GMS_MASK;
+
+	switch (gms) {
+	case I830_GMCH_GMS_STOLEN_512:	return KB(512);
+	case I830_GMCH_GMS_STOLEN_1024:	return MB(1);
+	case I830_GMCH_GMS_STOLEN_8192:	return MB(8);
+	/* local memory isn't part of the normal address space */
+	case I830_GMCH_GMS_LOCAL:	return 0;
 	default:
-		return 0;
+		WARN(1, "Unknown register value!\n");
 	}
 
-	return stolen_size;
+	return 0;
 }
 
 static size_t __init gen3_stolen_size(int num, int slot, int func)
 {
-	size_t stolen_size;
 	u16 gmch_ctrl;
+	u16 gms;
 
 	gmch_ctrl = read_pci_config_16(0, 0, 0, I830_GMCH_CTRL);
-
-	switch (gmch_ctrl & I855_GMCH_GMS_MASK) {
-	case I855_GMCH_GMS_STOLEN_1M:
-		stolen_size = MB(1);
-		break;
-	case I855_GMCH_GMS_STOLEN_4M:
-		stolen_size = MB(4);
-		break;
-	case I855_GMCH_GMS_STOLEN_8M:
-		stolen_size = MB(8);
-		break;
-	case I855_GMCH_GMS_STOLEN_16M:
-		stolen_size = MB(16);
-		break;
-	case I855_GMCH_GMS_STOLEN_32M:
-		stolen_size = MB(32);
-		break;
-	case I915_GMCH_GMS_STOLEN_48M:
-		stolen_size = MB(48);
-		break;
-	case I915_GMCH_GMS_STOLEN_64M:
-		stolen_size = MB(64);
-		break;
-	case G33_GMCH_GMS_STOLEN_128M:
-		stolen_size = MB(128);
-		break;
-	case G33_GMCH_GMS_STOLEN_256M:
-		stolen_size = MB(256);
-		break;
-	case INTEL_GMCH_GMS_STOLEN_96M:
-		stolen_size = MB(96);
-		break;
-	case INTEL_GMCH_GMS_STOLEN_160M:
-		stolen_size = MB(160);
-		break;
-	case INTEL_GMCH_GMS_STOLEN_224M:
-		stolen_size = MB(224);
-		break;
-	case INTEL_GMCH_GMS_STOLEN_352M:
-		stolen_size = MB(352);
-		break;
+	gms = gmch_ctrl & I855_GMCH_GMS_MASK;
+
+	switch (gms) {
+	case I855_GMCH_GMS_STOLEN_1M:	return MB(1);
+	case I855_GMCH_GMS_STOLEN_4M:	return MB(4);
+	case I855_GMCH_GMS_STOLEN_8M:	return MB(8);
+	case I855_GMCH_GMS_STOLEN_16M:	return MB(16);
+	case I855_GMCH_GMS_STOLEN_32M:	return MB(32);
+	case I915_GMCH_GMS_STOLEN_48M:	return MB(48);
+	case I915_GMCH_GMS_STOLEN_64M:	return MB(64);
+	case G33_GMCH_GMS_STOLEN_128M:	return MB(128);
+	case G33_GMCH_GMS_STOLEN_256M:	return MB(256);
+	case INTEL_GMCH_GMS_STOLEN_96M:	return MB(96);
+	case INTEL_GMCH_GMS_STOLEN_160M:return MB(160);
+	case INTEL_GMCH_GMS_STOLEN_224M:return MB(224);
+	case INTEL_GMCH_GMS_STOLEN_352M:return MB(352);
 	default:
-		stolen_size = 0;
-		break;
+		WARN(1, "Unknown register value!\n");
 	}
 
-	return stolen_size;
+	return 0;
 }
 
 static size_t __init gen6_stolen_size(int num, int slot, int func)
 {
 	u16 gmch_ctrl;
+	u16 gms;
 
 	gmch_ctrl = read_pci_config_16(num, slot, func, SNB_GMCH_CTRL);
-	gmch_ctrl >>= SNB_GMCH_GMS_SHIFT;
-	gmch_ctrl &= SNB_GMCH_GMS_MASK;
+	gms = (gmch_ctrl >> SNB_GMCH_GMS_SHIFT) & SNB_GMCH_GMS_MASK;
 
-	return gmch_ctrl << 25; /* 32 MB units */
+	return (size_t)gms * MB(32);
 }
 
 static size_t __init gen8_stolen_size(int num, int slot, int func)
 {
 	u16 gmch_ctrl;
+	u16 gms;
 
 	gmch_ctrl = read_pci_config_16(num, slot, func, SNB_GMCH_CTRL);
-	gmch_ctrl >>= BDW_GMCH_GMS_SHIFT;
-	gmch_ctrl &= BDW_GMCH_GMS_MASK;
-	return gmch_ctrl << 25; /* 32 MB units */
+	gms = (gmch_ctrl >> BDW_GMCH_GMS_SHIFT) & BDW_GMCH_GMS_MASK;
+
+	return (size_t)gms * MB(32);
 }
 
 static size_t __init chv_stolen_size(int num, int slot, int func)
 {
 	u16 gmch_ctrl;
+	u16 gms;
 
 	gmch_ctrl = read_pci_config_16(num, slot, func, SNB_GMCH_CTRL);
-	gmch_ctrl >>= SNB_GMCH_GMS_SHIFT;
-	gmch_ctrl &= SNB_GMCH_GMS_MASK;
+	gms = (gmch_ctrl >> SNB_GMCH_GMS_SHIFT) & SNB_GMCH_GMS_MASK;
 
 	/*
 	 * 0x0  to 0x10: 32MB increments starting at 0MB
 	 * 0x11 to 0x16: 4MB increments starting at 8MB
 	 * 0x17 to 0x1d: 4MB increments start at 36MB
 	 */
-	if (gmch_ctrl < 0x11)
-		return gmch_ctrl << 25;
-	else if (gmch_ctrl < 0x17)
-		return (gmch_ctrl - 0x11 + 2) << 22;
+	if (gms < 0x11)
+		return (size_t)gms * MB(32);
+	else if (gms < 0x17)
+		return (size_t)(gms - 0x11 + 2) * MB(4);
 	else
-		return (gmch_ctrl - 0x17 + 9) << 22;
+		return (size_t)(gms - 0x17 + 9) * MB(4);
 }
 
 struct intel_stolen_funcs {
 	size_t (*size)(int num, int slot, int func);
-	u32 (*base)(int num, int slot, int func, size_t size);
+	phys_addr_t (*base)(int num, int slot, int func, size_t size);
 };
 
 static size_t __init gen9_stolen_size(int num, int slot, int func)
 {
 	u16 gmch_ctrl;
+	u16 gms;
 
 	gmch_ctrl = read_pci_config_16(num, slot, func, SNB_GMCH_CTRL);
-	gmch_ctrl >>= BDW_GMCH_GMS_SHIFT;
-	gmch_ctrl &= BDW_GMCH_GMS_MASK;
+	gms = (gmch_ctrl >> BDW_GMCH_GMS_SHIFT) & BDW_GMCH_GMS_MASK;
 
-	if (gmch_ctrl < 0xf0)
-		return gmch_ctrl << 25; /* 32 MB units */
+	/* 0x0  to 0xef: 32MB increments starting at 0MB */
+	/* 0xf0 to 0xfe: 4MB increments starting at 4MB */
+	if (gms < 0xf0)
+		return (size_t)gms * MB(32);
 	else
-		/* 4MB increments starting at 0xf0 for 4MB */
-		return (gmch_ctrl - 0xf0 + 1) << 22;
+		return (size_t)(gms - 0xf0 + 1) * MB(4);
 }
 
 typedef size_t (*stolen_size_fn)(int num, int slot, int func);
@@ -493,27 +467,27 @@ static const struct intel_stolen_funcs i865_stolen_funcs __initconst = {
 };
 
 static const struct intel_stolen_funcs gen3_stolen_funcs __initconst = {
-	.base = intel_stolen_base,
+	.base = gen3_stolen_base,
 	.size = gen3_stolen_size,
 };
 
 static const struct intel_stolen_funcs gen6_stolen_funcs __initconst = {
-	.base = intel_stolen_base,
+	.base = gen3_stolen_base,
 	.size = gen6_stolen_size,
 };
 
 static const struct intel_stolen_funcs gen8_stolen_funcs __initconst = {
-	.base = intel_stolen_base,
+	.base = gen3_stolen_base,
 	.size = gen8_stolen_size,
 };
 
 static const struct intel_stolen_funcs gen9_stolen_funcs __initconst = {
-	.base = intel_stolen_base,
+	.base = gen3_stolen_base,
 	.size = gen9_stolen_size,
 };
 
 static const struct intel_stolen_funcs chv_stolen_funcs __initconst = {
-	.base = intel_stolen_base,
+	.base = gen3_stolen_base,
 	.size = chv_stolen_size,
 };
 
@@ -554,7 +528,7 @@ static void __init intel_graphics_stolen(int num, int slot, int func)
 {
 	size_t size;
 	int i;
-	u32 start;
+	phys_addr_t start;
 	u16 device, subvendor, subdevice;
 
 	device = read_pci_config_16(num, slot, func, PCI_DEVICE_ID);
@@ -569,8 +543,8 @@ static void __init intel_graphics_stolen(int num, int slot, int func)
 			size = stolen_funcs->size(num, slot, func);
 			start = stolen_funcs->base(num, slot, func, size);
 			if (size && start) {
-				printk(KERN_INFO "Reserving Intel graphics stolen memory at 0x%x-0x%x\n",
-				       start, start + (u32)size - 1);
+				printk(KERN_INFO "Reserving Intel graphics stolen memory at 0x%llx-0x%llx\n",
+				       start, start + size - 1);
 				/* Mark this space as reserved */
 				e820_add_region(start, size, E820_RESERVED);
 				sanitize_e820_map(e820.map,
diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
index b7ce963..68fde8f 100644
--- a/drivers/gpu/drm/i915/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
@@ -109,9 +109,9 @@ static unsigned long i915_stolen_to_physical(struct drm_device *dev)
 	if (INTEL_INFO(dev)->gen >= 3) {
 		u32 bsm;
 
-		pci_read_config_dword(dev->pdev, BSM, &bsm);
+		pci_read_config_dword(dev->pdev, INTEL_BSM, &bsm);
 
-		base = bsm & BSM_MASK;
+		base = bsm & INTEL_BSM_MASK;
 	} else if (IS_I865G(dev)) {
 		u16 toud = 0;
 
diff --git a/include/drm/i915_drm.h b/include/drm/i915_drm.h
index 595f85c..b1755f8 100644
--- a/include/drm/i915_drm.h
+++ b/include/drm/i915_drm.h
@@ -92,4 +92,7 @@ extern bool i915_gpu_turbo_disable(void);
 #define    I845_TSEG_SIZE_512K	(2 << 1)
 #define    I845_TSEG_SIZE_1M	(3 << 1)
 
+#define INTEL_BSM 0x5c
+#define   INTEL_BSM_MASK (0xFFFF << 20)
+
 #endif				/* _I915_DRM_H_ */
-- 
2.5.5

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

Re: [PATCH 1/2] drm/i915: Canonicalize stolen memory calculations

[Chris Wilson]

On Fri, Apr 22, 2016 at 02:25:06PM +0300, Joonas Lahtinen wrote:
> Move the better constructs/comments from i915_gem_stolen.c to
> early-quirks.c and increase readability in preparation of only
> having one set of functions.
> 
> - intel_stolen_base -> gen3_stolen_base
> - use phys_addr_t instead of u32 for address for future proofing
> 
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
> Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

Looks like a fairly mechanical change, so
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

>  static size_t __init i830_stolen_size(int num, int slot, int func)
>  {
> -	size_t stolen_size;
>  	u16 gmch_ctrl;
> +	u16 gms;
>  
>  	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;
> +	gms = gmch_ctrl & I830_GMCH_GMS_MASK;
> +
> +	switch (gms) {
> +	case I830_GMCH_GMS_STOLEN_512:	return KB(512);
> +	case I830_GMCH_GMS_STOLEN_1024:	return MB(1);
> +	case I830_GMCH_GMS_STOLEN_8192:	return MB(8);
> +	/* local memory isn't part of the normal address space */
> +	case I830_GMCH_GMS_LOCAL:	return 0;
>  	default:
> -		return 0;
> +		WARN(1, "Unknown register value!\n");

When adding a warn try to include the pertinent information:

WARN(1, "Unknown I830_GMCH_CTRL value: %x!\n", gms);
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915: Canonicalize stolen memory calculations

[Joonas Lahtinen]

Hi,

Adding x86 maintainers as CC as they were missing from the original
patch due to some confusion in best practices.

This patch was merged to the drm-intel.git after reviewing and testing.

Regards, Joonas

On pe, 2016-04-22 at 14:25 +0300, Joonas Lahtinen wrote:
> Move the better constructs/comments from i915_gem_stolen.c to
> early-quirks.c and increase readability in preparation of only
> having one set of functions.
> 
> - intel_stolen_base -> gen3_stolen_base
> - use phys_addr_t instead of u32 for address for future proofing
> 
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
> Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> ---
>  arch/x86/kernel/early-quirks.c         | 234 +++++++++++++++------------------
>  drivers/gpu/drm/i915/i915_gem_stolen.c |   4 +-
>  include/drm/i915_drm.h                 |   3 +
>  3 files changed, 109 insertions(+), 132 deletions(-)
> 
> diff --git a/arch/x86/kernel/early-quirks.c b/arch/x86/kernel/early-quirks.c
> index bca14c8..f169475 100644
> --- a/arch/x86/kernel/early-quirks.c
> +++ b/arch/x86/kernel/early-quirks.c
> @@ -223,36 +223,19 @@ static void __init intel_remapping_check(int num, int slot, int func)
>   * despite the efforts of the "RAM buffer" approach, which simply rounds
>   * memory boundaries up to 64M to try to catch space that may decode
>   * as RAM and so is not suitable for MMIO.
> - *
> - * 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, size_t stolen_size)
> -{
> -	u32 base;
> -
> -	/*
> -	 * For the PCI IDs in this quirk, the stolen base is always
> -	 * in 0x5c, aka the BDSM register (yes that's really what
> -	 * it's called).
> -	 */
> -	base = read_pci_config(num, slot, func, 0x5c);
> -	base &= ~((1<<20) - 1);
> -
> -	return base;
> -}
>  
>  #define KB(x)	((x) * 1024UL)
>  #define MB(x)	(KB (KB (x)))
> -#define GB(x)	(MB (KB (x)))
>  
>  static size_t __init i830_tseg_size(void)
>  {
> -	u8 tmp = read_pci_config_byte(0, 0, 0, I830_ESMRAMC);
> +	u8 esmramc = read_pci_config_byte(0, 0, 0, I830_ESMRAMC);
>  
> -	if (!(tmp & TSEG_ENABLE))
> +	if (!(esmramc & TSEG_ENABLE))
>  		return 0;
>  
> -	if (tmp & I830_TSEG_SIZE_1M)
> +	if (esmramc & I830_TSEG_SIZE_1M)
>  		return MB(1);
>  	else
>  		return KB(512);
> @@ -260,27 +243,26 @@ static size_t __init i830_tseg_size(void)
>  
>  static size_t __init i845_tseg_size(void)
>  {
> -	u8 tmp = read_pci_config_byte(0, 0, 0, I845_ESMRAMC);
> +	u8 esmramc = read_pci_config_byte(0, 0, 0, I845_ESMRAMC);
> +	u8 tseg_size = esmramc & I845_TSEG_SIZE_MASK;
>  
> -	if (!(tmp & TSEG_ENABLE))
> +	if (!(esmramc & TSEG_ENABLE))
>  		return 0;
>  
> -	switch (tmp & I845_TSEG_SIZE_MASK) {
> -	case I845_TSEG_SIZE_512K:
> -		return KB(512);
> -	case I845_TSEG_SIZE_1M:
> -		return MB(1);
> +	switch (tseg_size) {
> +	case I845_TSEG_SIZE_512K:	return KB(512);
> +	case I845_TSEG_SIZE_1M:		return MB(1);
>  	default:
> -		WARN_ON(1);
> -		return 0;
> +		WARN(1, "Unknown register value!\n");
>  	}
> +	return 0;
>  }
>  
>  static size_t __init i85x_tseg_size(void)
>  {
> -	u8 tmp = read_pci_config_byte(0, 0, 0, I85X_ESMRAMC);
> +	u8 esmramc = read_pci_config_byte(0, 0, 0, I85X_ESMRAMC);
>  
> -	if (!(tmp & TSEG_ENABLE))
> +	if (!(esmramc & TSEG_ENABLE))
>  		return 0;
>  
>  	return MB(1);
> @@ -300,174 +282,166 @@ static size_t __init i85x_mem_size(void)
>   * On 830/845/85x the stolen memory base isn't available in any
>   * register. We need to calculate it as TOM-TSEG_SIZE-stolen_size.
>   */
> -static u32 __init i830_stolen_base(int num, int slot, int func, size_t stolen_size)
> +static phys_addr_t __init i830_stolen_base(int num, int slot, int func,
> +					   size_t stolen_size)
>  {
> -	return i830_mem_size() - i830_tseg_size() - stolen_size;
> +	return (phys_addr_t)i830_mem_size() - i830_tseg_size() - stolen_size;
>  }
>  
> -static u32 __init i845_stolen_base(int num, int slot, int func, size_t stolen_size)
> +static phys_addr_t __init i845_stolen_base(int num, int slot, int func,
> +					   size_t stolen_size)
>  {
> -	return i830_mem_size() - i845_tseg_size() - stolen_size;
> +	return (phys_addr_t)i830_mem_size() - i845_tseg_size() - stolen_size;
>  }
>  
> -static u32 __init i85x_stolen_base(int num, int slot, int func, size_t stolen_size)
> +static phys_addr_t __init i85x_stolen_base(int num, int slot, int func,
> +					   size_t stolen_size)
>  {
> -	return i85x_mem_size() - i85x_tseg_size() - stolen_size;
> +	return (phys_addr_t)i85x_mem_size() - i85x_tseg_size() - stolen_size;
>  }
>  
> -static u32 __init i865_stolen_base(int num, int slot, int func, size_t stolen_size)
> +static phys_addr_t __init i865_stolen_base(int num, int slot, int func,
> +					   size_t stolen_size)
>  {
> +	u16 toud;
> +
>  	/*
>  	 * FIXME is the graphics stolen memory region
>  	 * always at TOUD? Ie. is it always the last
>  	 * one to be allocated by the BIOS?
>  	 */
> -	return read_pci_config_16(0, 0, 0, I865_TOUD) << 16;
> +	toud = read_pci_config_16(0, 0, 0, I865_TOUD);
> +
> +	return (phys_addr_t)toud << 16;
> +}
> +
> +static phys_addr_t __init gen3_stolen_base(int num, int slot, int func,
> +					   size_t stolen_size)
> +{
> +	u32 bsm;
> +
> +	/* Almost universally we can find the Graphics Base of Stolen Memory
> +	 * at register BSM (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.
> +	 */
> +	bsm = read_pci_config(num, slot, func, INTEL_BSM);
> +
> +	return (phys_addr_t)bsm & INTEL_BSM_MASK;
>  }
>  
>  static size_t __init i830_stolen_size(int num, int slot, int func)
>  {
> -	size_t stolen_size;
>  	u16 gmch_ctrl;
> +	u16 gms;
>  
>  	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;
> +	gms = gmch_ctrl & I830_GMCH_GMS_MASK;
> +
> +	switch (gms) {
> +	case I830_GMCH_GMS_STOLEN_512:	return KB(512);
> +	case I830_GMCH_GMS_STOLEN_1024:	return MB(1);
> +	case I830_GMCH_GMS_STOLEN_8192:	return MB(8);
> +	/* local memory isn't part of the normal address space */
> +	case I830_GMCH_GMS_LOCAL:	return 0;
>  	default:
> -		return 0;
> +		WARN(1, "Unknown register value!\n");
>  	}
>  
> -	return stolen_size;
> +	return 0;
>  }
>  
>  static size_t __init gen3_stolen_size(int num, int slot, int func)
>  {
> -	size_t stolen_size;
>  	u16 gmch_ctrl;
> +	u16 gms;
>  
>  	gmch_ctrl = read_pci_config_16(0, 0, 0, I830_GMCH_CTRL);
> -
> -	switch (gmch_ctrl & I855_GMCH_GMS_MASK) {
> -	case I855_GMCH_GMS_STOLEN_1M:
> -		stolen_size = MB(1);
> -		break;
> -	case I855_GMCH_GMS_STOLEN_4M:
> -		stolen_size = MB(4);
> -		break;
> -	case I855_GMCH_GMS_STOLEN_8M:
> -		stolen_size = MB(8);
> -		break;
> -	case I855_GMCH_GMS_STOLEN_16M:
> -		stolen_size = MB(16);
> -		break;
> -	case I855_GMCH_GMS_STOLEN_32M:
> -		stolen_size = MB(32);
> -		break;
> -	case I915_GMCH_GMS_STOLEN_48M:
> -		stolen_size = MB(48);
> -		break;
> -	case I915_GMCH_GMS_STOLEN_64M:
> -		stolen_size = MB(64);
> -		break;
> -	case G33_GMCH_GMS_STOLEN_128M:
> -		stolen_size = MB(128);
> -		break;
> -	case G33_GMCH_GMS_STOLEN_256M:
> -		stolen_size = MB(256);
> -		break;
> -	case INTEL_GMCH_GMS_STOLEN_96M:
> -		stolen_size = MB(96);
> -		break;
> -	case INTEL_GMCH_GMS_STOLEN_160M:
> -		stolen_size = MB(160);
> -		break;
> -	case INTEL_GMCH_GMS_STOLEN_224M:
> -		stolen_size = MB(224);
> -		break;
> -	case INTEL_GMCH_GMS_STOLEN_352M:
> -		stolen_size = MB(352);
> -		break;
> +	gms = gmch_ctrl & I855_GMCH_GMS_MASK;
> +
> +	switch (gms) {
> +	case I855_GMCH_GMS_STOLEN_1M:	return MB(1);
> +	case I855_GMCH_GMS_STOLEN_4M:	return MB(4);
> +	case I855_GMCH_GMS_STOLEN_8M:	return MB(8);
> +	case I855_GMCH_GMS_STOLEN_16M:	return MB(16);
> +	case I855_GMCH_GMS_STOLEN_32M:	return MB(32);
> +	case I915_GMCH_GMS_STOLEN_48M:	return MB(48);
> +	case I915_GMCH_GMS_STOLEN_64M:	return MB(64);
> +	case G33_GMCH_GMS_STOLEN_128M:	return MB(128);
> +	case G33_GMCH_GMS_STOLEN_256M:	return MB(256);
> +	case INTEL_GMCH_GMS_STOLEN_96M:	return MB(96);
> +	case INTEL_GMCH_GMS_STOLEN_160M:return MB(160);
> +	case INTEL_GMCH_GMS_STOLEN_224M:return MB(224);
> +	case INTEL_GMCH_GMS_STOLEN_352M:return MB(352);
>  	default:
> -		stolen_size = 0;
> -		break;
> +		WARN(1, "Unknown register value!\n");
>  	}
>  
> -	return stolen_size;
> +	return 0;
>  }
>  
>  static size_t __init gen6_stolen_size(int num, int slot, int func)
>  {
>  	u16 gmch_ctrl;
> +	u16 gms;
>  
>  	gmch_ctrl = read_pci_config_16(num, slot, func, SNB_GMCH_CTRL);
> -	gmch_ctrl >>= SNB_GMCH_GMS_SHIFT;
> -	gmch_ctrl &= SNB_GMCH_GMS_MASK;
> +	gms = (gmch_ctrl >> SNB_GMCH_GMS_SHIFT) & SNB_GMCH_GMS_MASK;
>  
> -	return gmch_ctrl << 25; /* 32 MB units */
> +	return (size_t)gms * MB(32);
>  }
>  
>  static size_t __init gen8_stolen_size(int num, int slot, int func)
>  {
>  	u16 gmch_ctrl;
> +	u16 gms;
>  
>  	gmch_ctrl = read_pci_config_16(num, slot, func, SNB_GMCH_CTRL);
> -	gmch_ctrl >>= BDW_GMCH_GMS_SHIFT;
> -	gmch_ctrl &= BDW_GMCH_GMS_MASK;
> -	return gmch_ctrl << 25; /* 32 MB units */
> +	gms = (gmch_ctrl >> BDW_GMCH_GMS_SHIFT) & BDW_GMCH_GMS_MASK;
> +
> +	return (size_t)gms * MB(32);
>  }
>  
>  static size_t __init chv_stolen_size(int num, int slot, int func)
>  {
>  	u16 gmch_ctrl;
> +	u16 gms;
>  
>  	gmch_ctrl = read_pci_config_16(num, slot, func, SNB_GMCH_CTRL);
> -	gmch_ctrl >>= SNB_GMCH_GMS_SHIFT;
> -	gmch_ctrl &= SNB_GMCH_GMS_MASK;
> +	gms = (gmch_ctrl >> SNB_GMCH_GMS_SHIFT) & SNB_GMCH_GMS_MASK;
>  
>  	/*
>  	 * 0x0  to 0x10: 32MB increments starting at 0MB
>  	 * 0x11 to 0x16: 4MB increments starting at 8MB
>  	 * 0x17 to 0x1d: 4MB increments start at 36MB
>  	 */
> -	if (gmch_ctrl < 0x11)
> -		return gmch_ctrl << 25;
> -	else if (gmch_ctrl < 0x17)
> -		return (gmch_ctrl - 0x11 + 2) << 22;
> +	if (gms < 0x11)
> +		return (size_t)gms * MB(32);
> +	else if (gms < 0x17)
> +		return (size_t)(gms - 0x11 + 2) * MB(4);
>  	else
> -		return (gmch_ctrl - 0x17 + 9) << 22;
> +		return (size_t)(gms - 0x17 + 9) * MB(4);
>  }
>  
>  struct intel_stolen_funcs {
>  	size_t (*size)(int num, int slot, int func);
> -	u32 (*base)(int num, int slot, int func, size_t size);
> +	phys_addr_t (*base)(int num, int slot, int func, size_t size);
>  };
>  
>  static size_t __init gen9_stolen_size(int num, int slot, int func)
>  {
>  	u16 gmch_ctrl;
> +	u16 gms;
>  
>  	gmch_ctrl = read_pci_config_16(num, slot, func, SNB_GMCH_CTRL);
> -	gmch_ctrl >>= BDW_GMCH_GMS_SHIFT;
> -	gmch_ctrl &= BDW_GMCH_GMS_MASK;
> +	gms = (gmch_ctrl >> BDW_GMCH_GMS_SHIFT) & BDW_GMCH_GMS_MASK;
>  
> -	if (gmch_ctrl < 0xf0)
> -		return gmch_ctrl << 25; /* 32 MB units */
> +	/* 0x0  to 0xef: 32MB increments starting at 0MB */
> +	/* 0xf0 to 0xfe: 4MB increments starting at 4MB */
> +	if (gms < 0xf0)
> +		return (size_t)gms * MB(32);
>  	else
> -		/* 4MB increments starting at 0xf0 for 4MB */
> -		return (gmch_ctrl - 0xf0 + 1) << 22;
> +		return (size_t)(gms - 0xf0 + 1) * MB(4);
>  }
>  
>  typedef size_t (*stolen_size_fn)(int num, int slot, int func);
> @@ -493,27 +467,27 @@ static const struct intel_stolen_funcs i865_stolen_funcs __initconst = {
>  };
>  
>  static const struct intel_stolen_funcs gen3_stolen_funcs __initconst = {
> -	.base = intel_stolen_base,
> +	.base = gen3_stolen_base,
>  	.size = gen3_stolen_size,
>  };
>  
>  static const struct intel_stolen_funcs gen6_stolen_funcs __initconst = {
> -	.base = intel_stolen_base,
> +	.base = gen3_stolen_base,
>  	.size = gen6_stolen_size,
>  };
>  
>  static const struct intel_stolen_funcs gen8_stolen_funcs __initconst = {
> -	.base = intel_stolen_base,
> +	.base = gen3_stolen_base,
>  	.size = gen8_stolen_size,
>  };
>  
>  static const struct intel_stolen_funcs gen9_stolen_funcs __initconst = {
> -	.base = intel_stolen_base,
> +	.base = gen3_stolen_base,
>  	.size = gen9_stolen_size,
>  };
>  
>  static const struct intel_stolen_funcs chv_stolen_funcs __initconst = {
> -	.base = intel_stolen_base,
> +	.base = gen3_stolen_base,
>  	.size = chv_stolen_size,
>  };
>  
> @@ -554,7 +528,7 @@ static void __init intel_graphics_stolen(int num, int slot, int func)
>  {
>  	size_t size;
>  	int i;
> -	u32 start;
> +	phys_addr_t start;
>  	u16 device, subvendor, subdevice;
>  
>  	device = read_pci_config_16(num, slot, func, PCI_DEVICE_ID);
> @@ -569,8 +543,8 @@ static void __init intel_graphics_stolen(int num, int slot, int func)
>  			size = stolen_funcs->size(num, slot, func);
>  			start = stolen_funcs->base(num, slot, func, size);
>  			if (size && start) {
> -				printk(KERN_INFO "Reserving Intel graphics stolen memory at 0x%x-0x%x\n",
> -				       start, start + (u32)size - 1);
> +				printk(KERN_INFO "Reserving Intel graphics stolen memory at 0x%llx-0x%llx\n",
> +				       start, start + size - 1);
>  				/* Mark this space as reserved */
>  				e820_add_region(start, size, E820_RESERVED);
>  				sanitize_e820_map(e820.map,
> diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
> index b7ce963..68fde8f 100644
> --- a/drivers/gpu/drm/i915/i915_gem_stolen.c
> +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
> @@ -109,9 +109,9 @@ static unsigned long i915_stolen_to_physical(struct drm_device *dev)
>  	if (INTEL_INFO(dev)->gen >= 3) {
>  		u32 bsm;
>  
> -		pci_read_config_dword(dev->pdev, BSM, &bsm);
> +		pci_read_config_dword(dev->pdev, INTEL_BSM, &bsm);
>  
> -		base = bsm & BSM_MASK;
> +		base = bsm & INTEL_BSM_MASK;
>  	} else if (IS_I865G(dev)) {
>  		u16 toud = 0;
>  
> diff --git a/include/drm/i915_drm.h b/include/drm/i915_drm.h
> index 595f85c..b1755f8 100644
> --- a/include/drm/i915_drm.h
> +++ b/include/drm/i915_drm.h
> @@ -92,4 +92,7 @@ extern bool i915_gpu_turbo_disable(void);
>  #define    I845_TSEG_SIZE_512K	(2 << 1)
>  #define    I845_TSEG_SIZE_1M	(3 << 1)
>  
> +#define INTEL_BSM 0x5c
> +#define   INTEL_BSM_MASK (0xFFFF << 20)
> +
>  #endif				/* _I915_DRM_H_ */
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/2] drm/i915: Function per early graphics quirk

[Joonas Lahtinen]

Move graphics stolen memory related early quirk into a function to
allow easy adding of other graphics quirks to fix memory maps on
machines running old BIOS versions.

While at it;
- _funcs -> _ops to follow de facto naming
- make the iteration code tad more readable
- remove unused variables

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 arch/x86/kernel/early-quirks.c | 187 +++++++++++++++++++++--------------------
 1 file changed, 98 insertions(+), 89 deletions(-)

diff --git a/arch/x86/kernel/early-quirks.c b/arch/x86/kernel/early-quirks.c
index f169475..bef1376 100644
--- a/arch/x86/kernel/early-quirks.c
+++ b/arch/x86/kernel/early-quirks.c
@@ -423,11 +423,6 @@ static size_t __init chv_stolen_size(int num, int slot, int func)
 		return (size_t)(gms - 0x17 + 9) * MB(4);
 }
 
-struct intel_stolen_funcs {
-	size_t (*size)(int num, int slot, int func);
-	phys_addr_t (*base)(int num, int slot, int func, size_t size);
-};
-
 static size_t __init gen9_stolen_size(int num, int slot, int func)
 {
 	u16 gmch_ctrl;
@@ -444,115 +439,129 @@ static size_t __init gen9_stolen_size(int num, int slot, int func)
 		return (size_t)(gms - 0xf0 + 1) * MB(4);
 }
 
-typedef size_t (*stolen_size_fn)(int num, int slot, int func);
+struct intel_early_ops {
+	size_t (*stolen_size)(int num, int slot, int func);
+	phys_addr_t (*stolen_base)(int num, int slot, int func, size_t size);
+};
 
-static const struct intel_stolen_funcs i830_stolen_funcs __initconst = {
-	.base = i830_stolen_base,
-	.size = i830_stolen_size,
+static const struct intel_early_ops i830_early_ops __initconst = {
+	.stolen_base = i830_stolen_base,
+	.stolen_size = i830_stolen_size,
 };
 
-static const struct intel_stolen_funcs i845_stolen_funcs __initconst = {
-	.base = i845_stolen_base,
-	.size = i830_stolen_size,
+static const struct intel_early_ops i845_early_ops __initconst = {
+	.stolen_base = i845_stolen_base,
+	.stolen_size = i830_stolen_size,
 };
 
-static const struct intel_stolen_funcs i85x_stolen_funcs __initconst = {
-	.base = i85x_stolen_base,
-	.size = gen3_stolen_size,
+static const struct intel_early_ops i85x_early_ops __initconst = {
+	.stolen_base = i85x_stolen_base,
+	.stolen_size = gen3_stolen_size,
 };
 
-static const struct intel_stolen_funcs i865_stolen_funcs __initconst = {
-	.base = i865_stolen_base,
-	.size = gen3_stolen_size,
+static const struct intel_early_ops i865_early_ops __initconst = {
+	.stolen_base = i865_stolen_base,
+	.stolen_size = gen3_stolen_size,
 };
 
-static const struct intel_stolen_funcs gen3_stolen_funcs __initconst = {
-	.base = gen3_stolen_base,
-	.size = gen3_stolen_size,
+static const struct intel_early_ops gen3_early_ops __initconst = {
+	.stolen_base = gen3_stolen_base,
+	.stolen_size = gen3_stolen_size,
 };
 
-static const struct intel_stolen_funcs gen6_stolen_funcs __initconst = {
-	.base = gen3_stolen_base,
-	.size = gen6_stolen_size,
+static const struct intel_early_ops gen6_early_ops __initconst = {
+	.stolen_base = gen3_stolen_base,
+	.stolen_size = gen6_stolen_size,
 };
 
-static const struct intel_stolen_funcs gen8_stolen_funcs __initconst = {
-	.base = gen3_stolen_base,
-	.size = gen8_stolen_size,
+static const struct intel_early_ops gen8_early_ops __initconst = {
+	.stolen_base = gen3_stolen_base,
+	.stolen_size = gen8_stolen_size,
 };
 
-static const struct intel_stolen_funcs gen9_stolen_funcs __initconst = {
-	.base = gen3_stolen_base,
-	.size = gen9_stolen_size,
+static const struct intel_early_ops gen9_early_ops __initconst = {
+	.stolen_base = gen3_stolen_base,
+	.stolen_size = gen9_stolen_size,
 };
 
-static const struct intel_stolen_funcs chv_stolen_funcs __initconst = {
-	.base = gen3_stolen_base,
-	.size = chv_stolen_size,
+static const struct intel_early_ops chv_early_ops __initconst = {
+	.stolen_base = gen3_stolen_base,
+	.stolen_size = chv_stolen_size,
 };
 
-static const struct pci_device_id intel_stolen_ids[] __initconst = {
-	INTEL_I830_IDS(&i830_stolen_funcs),
-	INTEL_I845G_IDS(&i845_stolen_funcs),
-	INTEL_I85X_IDS(&i85x_stolen_funcs),
-	INTEL_I865G_IDS(&i865_stolen_funcs),
-	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),
-	INTEL_CHV_IDS(&chv_stolen_funcs),
-	INTEL_SKL_IDS(&gen9_stolen_funcs),
-	INTEL_BXT_IDS(&gen9_stolen_funcs),
-	INTEL_KBL_IDS(&gen9_stolen_funcs),
+static const struct pci_device_id intel_early_ids[] __initconst = {
+	INTEL_I830_IDS(&i830_early_ops),
+	INTEL_I845G_IDS(&i845_early_ops),
+	INTEL_I85X_IDS(&i85x_early_ops),
+	INTEL_I865G_IDS(&i865_early_ops),
+	INTEL_I915G_IDS(&gen3_early_ops),
+	INTEL_I915GM_IDS(&gen3_early_ops),
+	INTEL_I945G_IDS(&gen3_early_ops),
+	INTEL_I945GM_IDS(&gen3_early_ops),
+	INTEL_VLV_M_IDS(&gen6_early_ops),
+	INTEL_VLV_D_IDS(&gen6_early_ops),
+	INTEL_PINEVIEW_IDS(&gen3_early_ops),
+	INTEL_I965G_IDS(&gen3_early_ops),
+	INTEL_G33_IDS(&gen3_early_ops),
+	INTEL_I965GM_IDS(&gen3_early_ops),
+	INTEL_GM45_IDS(&gen3_early_ops),
+	INTEL_G45_IDS(&gen3_early_ops),
+	INTEL_IRONLAKE_D_IDS(&gen3_early_ops),
+	INTEL_IRONLAKE_M_IDS(&gen3_early_ops),
+	INTEL_SNB_D_IDS(&gen6_early_ops),
+	INTEL_SNB_M_IDS(&gen6_early_ops),
+	INTEL_IVB_M_IDS(&gen6_early_ops),
+	INTEL_IVB_D_IDS(&gen6_early_ops),
+	INTEL_HSW_D_IDS(&gen6_early_ops),
+	INTEL_HSW_M_IDS(&gen6_early_ops),
+	INTEL_BDW_M_IDS(&gen8_early_ops),
+	INTEL_BDW_D_IDS(&gen8_early_ops),
+	INTEL_CHV_IDS(&chv_early_ops),
+	INTEL_SKL_IDS(&gen9_early_ops),
+	INTEL_BXT_IDS(&gen9_early_ops),
+	INTEL_KBL_IDS(&gen9_early_ops),
 };
 
-static void __init intel_graphics_stolen(int num, int slot, int func)
+static void __init
+intel_graphics_stolen(int num, int slot, int func,
+		      const struct intel_early_ops *early_ops)
 {
+	phys_addr_t base;
 	size_t size;
+
+	size = early_ops->stolen_size(num, slot, func);
+	base = early_ops->stolen_base(num, slot, func, size);
+
+	if (!size || !base)
+		return;
+
+	printk(KERN_INFO "Reserving Intel graphics stolen memory at "
+	       "0x%llx-0x%llx\n", base, base + size - 1);
+
+	/* Mark this space as reserved */
+	e820_add_region(base, size, E820_RESERVED);
+	sanitize_e820_map(e820.map, ARRAY_SIZE(e820.map), &e820.nr_map);
+}
+
+static void __init intel_graphics_quirks(int num, int slot, int func)
+{
+	const struct intel_early_ops *early_ops;
+	u16 device;
 	int i;
-	phys_addr_t start;
-	u16 device, subvendor, subdevice;
 
 	device = read_pci_config_16(num, slot, func, PCI_DEVICE_ID);
-	subvendor = read_pci_config_16(num, slot, func,
-				       PCI_SUBSYSTEM_VENDOR_ID);
-	subdevice = read_pci_config_16(num, slot, func, PCI_SUBSYSTEM_ID);
-
-	for (i = 0; i < ARRAY_SIZE(intel_stolen_ids); i++) {
-		if (intel_stolen_ids[i].device == device) {
-			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) {
-				printk(KERN_INFO "Reserving Intel graphics stolen memory at 0x%llx-0x%llx\n",
-				       start, start + size - 1);
-				/* Mark this space as reserved */
-				e820_add_region(start, size, E820_RESERVED);
-				sanitize_e820_map(e820.map,
-						  ARRAY_SIZE(e820.map),
-						  &e820.nr_map);
-			}
-			return;
-		}
+
+	for (i = 0; i < ARRAY_SIZE(intel_early_ids); i++) {
+		kernel_ulong_t driver_data = intel_early_ids[i].driver_data;
+
+		if (intel_early_ids[i].device != device)
+			continue;
+
+		early_ops = (typeof(early_ops))driver_data;
+
+		intel_graphics_stolen(num, slot, func, early_ops);
+
+		return;
 	}
 }
 
@@ -601,7 +610,7 @@ static struct chipset early_qrk[] __initdata = {
 	{ PCI_VENDOR_ID_INTEL, 0x3406, PCI_CLASS_BRIDGE_HOST,
 	  PCI_BASE_CLASS_BRIDGE, 0, intel_remapping_check },
 	{ PCI_VENDOR_ID_INTEL, PCI_ANY_ID, PCI_CLASS_DISPLAY_VGA, PCI_ANY_ID,
-	  QFLAG_APPLY_ONCE, intel_graphics_stolen },
+	  QFLAG_APPLY_ONCE, intel_graphics_quirks },
 	/*
 	 * HPET on the current version of the Baytrail platform has accuracy
 	 * problems: it will halt in deep idle state - so we disable it.
-- 
2.5.5

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

Re: [PATCH 2/2] drm/i915: Function per early graphics quirk

[Chris Wilson]

On Fri, Apr 22, 2016 at 02:25:07PM +0300, Joonas Lahtinen wrote:
> Move graphics stolen memory related early quirk into a function to
> allow easy adding of other graphics quirks to fix memory maps on
> machines running old BIOS versions.
> 
> While at it;
> - _funcs -> _ops to follow de facto naming
> - make the iteration code tad more readable
> - remove unused variables
> 
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
> Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

Mechanical again, and less quirky,
Reserved-by: Chris Wilson <chris@chris-wilson.co.uk>

> -static void __init intel_graphics_stolen(int num, int slot, int func)
> +static void __init
> +intel_graphics_stolen(int num, int slot, int func,
> +		      const struct intel_early_ops *early_ops)
>  {
> +	phys_addr_t base;
>  	size_t size;
> +
> +	size = early_ops->stolen_size(num, slot, func);
> +	base = early_ops->stolen_base(num, slot, func, size);
> +
> +	if (!size || !base)
> +		return;

if (size + base < base)
	return;

Just in case? Hmm, not sure if the integer sizes will always be suitable
for the test, so probably just trust the hardware registers?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/2] drm/i915: Function per early graphics quirk

[Joonas Lahtinen]

On pe, 2016-04-22 at 12:55 +0100, Chris Wilson wrote:
> On Fri, Apr 22, 2016 at 02:25:07PM +0300, Joonas Lahtinen wrote:
> > 
> > Move graphics stolen memory related early quirk into a function to
> > allow easy adding of other graphics quirks to fix memory maps on
> > machines running old BIOS versions.
> > 
> > While at it;
> > - _funcs -> _ops to follow de facto naming
> > - make the iteration code tad more readable
> > - remove unused variables
> > 
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
> > Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Mechanical again, and less quirky,
> Reserved-by: Chris Wilson <chris@chris-wilson.co.uk>

A new type of R-b? :)

> 
> > 
> > -static void __init intel_graphics_stolen(int num, int slot, int func)
> > +static void __init
> > +intel_graphics_stolen(int num, int slot, int func,
> > +		      const struct intel_early_ops *early_ops)
> >  {
> > +	phys_addr_t base;
> >  	size_t size;
> > +
> > +	size = early_ops->stolen_size(num, slot, func);
> > +	base = early_ops->stolen_base(num, slot, func, size);
> > +
> > +	if (!size || !base)
> > +		return;
> if (size + base < base)
> 	return;
> 
> Just in case? Hmm, not sure if the integer sizes will always be suitable
> for the test, so probably just trust the hardware registers?

(This was actually the test used by earlier version of code too.
Intention was to not change the resulting logic that much. So I'd add
this check as a follow-up.)

I don't see how this could happen, the original hardware registers are
unlikely to cause an overflow of the phys_addr_t. If we want to add a
check, we should rather test against e820_end_of_ram_pfn() or so.

Regards, Joonas

> -Chris
> 
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/2] drm/i915: Function per early graphics quirk

[Chris Wilson]

On Fri, Apr 22, 2016 at 03:07:42PM +0300, Joonas Lahtinen wrote:
> On pe, 2016-04-22 at 12:55 +0100, Chris Wilson wrote:
> > On Fri, Apr 22, 2016 at 02:25:07PM +0300, Joonas Lahtinen wrote:
> > > 
> > > Move graphics stolen memory related early quirk into a function to
> > > allow easy adding of other graphics quirks to fix memory maps on
> > > machines running old BIOS versions.
> > > 
> > > While at it;
> > > - _funcs -> _ops to follow de facto naming
> > > - make the iteration code tad more readable
> > > - remove unused variables
> > > 
> > > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > > Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
> > > Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > Mechanical again, and less quirky,
> > Reserved-by: Chris Wilson <chris@chris-wilson.co.uk>
> 
> A new type of R-b? :)

I'm tired. :|

> > > -static void __init intel_graphics_stolen(int num, int slot, int func)
> > > +static void __init
> > > +intel_graphics_stolen(int num, int slot, int func,
> > > +		      const struct intel_early_ops *early_ops)
> > >  {
> > > +	phys_addr_t base;
> > >  	size_t size;
> > > +
> > > +	size = early_ops->stolen_size(num, slot, func);
> > > +	base = early_ops->stolen_base(num, slot, func, size);
> > > +
> > > +	if (!size || !base)
> > > +		return;
> > if (size + base < base)
> > 	return;
> > 
> > Just in case? Hmm, not sure if the integer sizes will always be suitable
> > for the test, so probably just trust the hardware registers?
> 
> (This was actually the test used by earlier version of code too.
> Intention was to not change the resulting logic that much. So I'd add
> this check as a follow-up.)
> 
> I don't see how this could happen, the original hardware registers are
> unlikely to cause an overflow of the phys_addr_t. If we want to add a
> check, we should rather test against e820_end_of_ram_pfn() or so.

It can't happen. But I know better than to trust a BIOS or our own
interpretation of the guides. From off the top of my head, stolen will
always be below 4G, and doing a sanity check that it is may or may not
be worth the infuriation if it ever moves above 4G.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 1/2] drm/i915: Canonicalize stolen memory calculations

[Joonas Lahtinen]

Move the better constructs/comments from i915_gem_stolen.c to
early-quirks.c and increase readability in preparation of only
having one set of functions.

- intel_stolen_base -> gen3_stolen_base
- use phys_addr_t instead of u32 for address for future proofing

v2:
- Print the invalid register values (Chris)
  (Omitting the register prefix as it's visible from backtrace.)

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 arch/x86/kernel/early-quirks.c         | 234 +++++++++++++++------------------
 drivers/gpu/drm/i915/i915_gem_stolen.c |   4 +-
 include/drm/i915_drm.h                 |   3 +
 3 files changed, 109 insertions(+), 132 deletions(-)

diff --git a/arch/x86/kernel/early-quirks.c b/arch/x86/kernel/early-quirks.c
index bca14c8..70e0ab6 100644
--- a/arch/x86/kernel/early-quirks.c
+++ b/arch/x86/kernel/early-quirks.c
@@ -223,36 +223,19 @@ static void __init intel_remapping_check(int num, int slot, int func)
  * despite the efforts of the "RAM buffer" approach, which simply rounds
  * memory boundaries up to 64M to try to catch space that may decode
  * as RAM and so is not suitable for MMIO.
- *
- * 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, size_t stolen_size)
-{
-	u32 base;
-
-	/*
-	 * For the PCI IDs in this quirk, the stolen base is always
-	 * in 0x5c, aka the BDSM register (yes that's really what
-	 * it's called).
-	 */
-	base = read_pci_config(num, slot, func, 0x5c);
-	base &= ~((1<<20) - 1);
-
-	return base;
-}
 
 #define KB(x)	((x) * 1024UL)
 #define MB(x)	(KB (KB (x)))
-#define GB(x)	(MB (KB (x)))
 
 static size_t __init i830_tseg_size(void)
 {
-	u8 tmp = read_pci_config_byte(0, 0, 0, I830_ESMRAMC);
+	u8 esmramc = read_pci_config_byte(0, 0, 0, I830_ESMRAMC);
 
-	if (!(tmp & TSEG_ENABLE))
+	if (!(esmramc & TSEG_ENABLE))
 		return 0;
 
-	if (tmp & I830_TSEG_SIZE_1M)
+	if (esmramc & I830_TSEG_SIZE_1M)
 		return MB(1);
 	else
 		return KB(512);
@@ -260,27 +243,26 @@ static size_t __init i830_tseg_size(void)
 
 static size_t __init i845_tseg_size(void)
 {
-	u8 tmp = read_pci_config_byte(0, 0, 0, I845_ESMRAMC);
+	u8 esmramc = read_pci_config_byte(0, 0, 0, I845_ESMRAMC);
+	u8 tseg_size = esmramc & I845_TSEG_SIZE_MASK;
 
-	if (!(tmp & TSEG_ENABLE))
+	if (!(esmramc & TSEG_ENABLE))
 		return 0;
 
-	switch (tmp & I845_TSEG_SIZE_MASK) {
-	case I845_TSEG_SIZE_512K:
-		return KB(512);
-	case I845_TSEG_SIZE_1M:
-		return MB(1);
+	switch (tseg_size) {
+	case I845_TSEG_SIZE_512K:	return KB(512);
+	case I845_TSEG_SIZE_1M:		return MB(1);
 	default:
-		WARN_ON(1);
-		return 0;
+		WARN(1, "Unknown ESMRAMC value: %x!\n", esmramc);
 	}
+	return 0;
 }
 
 static size_t __init i85x_tseg_size(void)
 {
-	u8 tmp = read_pci_config_byte(0, 0, 0, I85X_ESMRAMC);
+	u8 esmramc = read_pci_config_byte(0, 0, 0, I85X_ESMRAMC);
 
-	if (!(tmp & TSEG_ENABLE))
+	if (!(esmramc & TSEG_ENABLE))
 		return 0;
 
 	return MB(1);
@@ -300,174 +282,166 @@ static size_t __init i85x_mem_size(void)
  * On 830/845/85x the stolen memory base isn't available in any
  * register. We need to calculate it as TOM-TSEG_SIZE-stolen_size.
  */
-static u32 __init i830_stolen_base(int num, int slot, int func, size_t stolen_size)
+static phys_addr_t __init i830_stolen_base(int num, int slot, int func,
+					   size_t stolen_size)
 {
-	return i830_mem_size() - i830_tseg_size() - stolen_size;
+	return (phys_addr_t)i830_mem_size() - i830_tseg_size() - stolen_size;
 }
 
-static u32 __init i845_stolen_base(int num, int slot, int func, size_t stolen_size)
+static phys_addr_t __init i845_stolen_base(int num, int slot, int func,
+					   size_t stolen_size)
 {
-	return i830_mem_size() - i845_tseg_size() - stolen_size;
+	return (phys_addr_t)i830_mem_size() - i845_tseg_size() - stolen_size;
 }
 
-static u32 __init i85x_stolen_base(int num, int slot, int func, size_t stolen_size)
+static phys_addr_t __init i85x_stolen_base(int num, int slot, int func,
+					   size_t stolen_size)
 {
-	return i85x_mem_size() - i85x_tseg_size() - stolen_size;
+	return (phys_addr_t)i85x_mem_size() - i85x_tseg_size() - stolen_size;
 }
 
-static u32 __init i865_stolen_base(int num, int slot, int func, size_t stolen_size)
+static phys_addr_t __init i865_stolen_base(int num, int slot, int func,
+					   size_t stolen_size)
 {
+	u16 toud;
+
 	/*
 	 * FIXME is the graphics stolen memory region
 	 * always at TOUD? Ie. is it always the last
 	 * one to be allocated by the BIOS?
 	 */
-	return read_pci_config_16(0, 0, 0, I865_TOUD) << 16;
+	toud = read_pci_config_16(0, 0, 0, I865_TOUD);
+
+	return (phys_addr_t)toud << 16;
+}
+
+static phys_addr_t __init gen3_stolen_base(int num, int slot, int func,
+					   size_t stolen_size)
+{
+	u32 bsm;
+
+	/* Almost universally we can find the Graphics Base of Stolen Memory
+	 * at register BSM (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.
+	 */
+	bsm = read_pci_config(num, slot, func, INTEL_BSM);
+
+	return (phys_addr_t)bsm & INTEL_BSM_MASK;
 }
 
 static size_t __init i830_stolen_size(int num, int slot, int func)
 {
-	size_t stolen_size;
 	u16 gmch_ctrl;
+	u16 gms;
 
 	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;
+	gms = gmch_ctrl & I830_GMCH_GMS_MASK;
+
+	switch (gms) {
+	case I830_GMCH_GMS_STOLEN_512:	return KB(512);
+	case I830_GMCH_GMS_STOLEN_1024:	return MB(1);
+	case I830_GMCH_GMS_STOLEN_8192:	return MB(8);
+	/* local memory isn't part of the normal address space */
+	case I830_GMCH_GMS_LOCAL:	return 0;
 	default:
-		return 0;
+		WARN(1, "Unknown GMCH_CTRL value: %x!\n", gmch_ctrl);
 	}
 
-	return stolen_size;
+	return 0;
 }
 
 static size_t __init gen3_stolen_size(int num, int slot, int func)
 {
-	size_t stolen_size;
 	u16 gmch_ctrl;
+	u16 gms;
 
 	gmch_ctrl = read_pci_config_16(0, 0, 0, I830_GMCH_CTRL);
-
-	switch (gmch_ctrl & I855_GMCH_GMS_MASK) {
-	case I855_GMCH_GMS_STOLEN_1M:
-		stolen_size = MB(1);
-		break;
-	case I855_GMCH_GMS_STOLEN_4M:
-		stolen_size = MB(4);
-		break;
-	case I855_GMCH_GMS_STOLEN_8M:
-		stolen_size = MB(8);
-		break;
-	case I855_GMCH_GMS_STOLEN_16M:
-		stolen_size = MB(16);
-		break;
-	case I855_GMCH_GMS_STOLEN_32M:
-		stolen_size = MB(32);
-		break;
-	case I915_GMCH_GMS_STOLEN_48M:
-		stolen_size = MB(48);
-		break;
-	case I915_GMCH_GMS_STOLEN_64M:
-		stolen_size = MB(64);
-		break;
-	case G33_GMCH_GMS_STOLEN_128M:
-		stolen_size = MB(128);
-		break;
-	case G33_GMCH_GMS_STOLEN_256M:
-		stolen_size = MB(256);
-		break;
-	case INTEL_GMCH_GMS_STOLEN_96M:
-		stolen_size = MB(96);
-		break;
-	case INTEL_GMCH_GMS_STOLEN_160M:
-		stolen_size = MB(160);
-		break;
-	case INTEL_GMCH_GMS_STOLEN_224M:
-		stolen_size = MB(224);
-		break;
-	case INTEL_GMCH_GMS_STOLEN_352M:
-		stolen_size = MB(352);
-		break;
+	gms = gmch_ctrl & I855_GMCH_GMS_MASK;
+
+	switch (gms) {
+	case I855_GMCH_GMS_STOLEN_1M:	return MB(1);
+	case I855_GMCH_GMS_STOLEN_4M:	return MB(4);
+	case I855_GMCH_GMS_STOLEN_8M:	return MB(8);
+	case I855_GMCH_GMS_STOLEN_16M:	return MB(16);
+	case I855_GMCH_GMS_STOLEN_32M:	return MB(32);
+	case I915_GMCH_GMS_STOLEN_48M:	return MB(48);
+	case I915_GMCH_GMS_STOLEN_64M:	return MB(64);
+	case G33_GMCH_GMS_STOLEN_128M:	return MB(128);
+	case G33_GMCH_GMS_STOLEN_256M:	return MB(256);
+	case INTEL_GMCH_GMS_STOLEN_96M:	return MB(96);
+	case INTEL_GMCH_GMS_STOLEN_160M:return MB(160);
+	case INTEL_GMCH_GMS_STOLEN_224M:return MB(224);
+	case INTEL_GMCH_GMS_STOLEN_352M:return MB(352);
 	default:
-		stolen_size = 0;
-		break;
+		WARN(1, "Unknown GMCH_CTRL value: %x!\n", gmch_ctrl);
 	}
 
-	return stolen_size;
+	return 0;
 }
 
 static size_t __init gen6_stolen_size(int num, int slot, int func)
 {
 	u16 gmch_ctrl;
+	u16 gms;
 
 	gmch_ctrl = read_pci_config_16(num, slot, func, SNB_GMCH_CTRL);
-	gmch_ctrl >>= SNB_GMCH_GMS_SHIFT;
-	gmch_ctrl &= SNB_GMCH_GMS_MASK;
+	gms = (gmch_ctrl >> SNB_GMCH_GMS_SHIFT) & SNB_GMCH_GMS_MASK;
 
-	return gmch_ctrl << 25; /* 32 MB units */
+	return (size_t)gms * MB(32);
 }
 
 static size_t __init gen8_stolen_size(int num, int slot, int func)
 {
 	u16 gmch_ctrl;
+	u16 gms;
 
 	gmch_ctrl = read_pci_config_16(num, slot, func, SNB_GMCH_CTRL);
-	gmch_ctrl >>= BDW_GMCH_GMS_SHIFT;
-	gmch_ctrl &= BDW_GMCH_GMS_MASK;
-	return gmch_ctrl << 25; /* 32 MB units */
+	gms = (gmch_ctrl >> BDW_GMCH_GMS_SHIFT) & BDW_GMCH_GMS_MASK;
+
+	return (size_t)gms * MB(32);
 }
 
 static size_t __init chv_stolen_size(int num, int slot, int func)
 {
 	u16 gmch_ctrl;
+	u16 gms;
 
 	gmch_ctrl = read_pci_config_16(num, slot, func, SNB_GMCH_CTRL);
-	gmch_ctrl >>= SNB_GMCH_GMS_SHIFT;
-	gmch_ctrl &= SNB_GMCH_GMS_MASK;
+	gms = (gmch_ctrl >> SNB_GMCH_GMS_SHIFT) & SNB_GMCH_GMS_MASK;
 
 	/*
 	 * 0x0  to 0x10: 32MB increments starting at 0MB
 	 * 0x11 to 0x16: 4MB increments starting at 8MB
 	 * 0x17 to 0x1d: 4MB increments start at 36MB
 	 */
-	if (gmch_ctrl < 0x11)
-		return gmch_ctrl << 25;
-	else if (gmch_ctrl < 0x17)
-		return (gmch_ctrl - 0x11 + 2) << 22;
+	if (gms < 0x11)
+		return (size_t)gms * MB(32);
+	else if (gms < 0x17)
+		return (size_t)(gms - 0x11 + 2) * MB(4);
 	else
-		return (gmch_ctrl - 0x17 + 9) << 22;
+		return (size_t)(gms - 0x17 + 9) * MB(4);
 }
 
 struct intel_stolen_funcs {
 	size_t (*size)(int num, int slot, int func);
-	u32 (*base)(int num, int slot, int func, size_t size);
+	phys_addr_t (*base)(int num, int slot, int func, size_t size);
 };
 
 static size_t __init gen9_stolen_size(int num, int slot, int func)
 {
 	u16 gmch_ctrl;
+	u16 gms;
 
 	gmch_ctrl = read_pci_config_16(num, slot, func, SNB_GMCH_CTRL);
-	gmch_ctrl >>= BDW_GMCH_GMS_SHIFT;
-	gmch_ctrl &= BDW_GMCH_GMS_MASK;
+	gms = (gmch_ctrl >> BDW_GMCH_GMS_SHIFT) & BDW_GMCH_GMS_MASK;
 
-	if (gmch_ctrl < 0xf0)
-		return gmch_ctrl << 25; /* 32 MB units */
+	/* 0x0  to 0xef: 32MB increments starting at 0MB */
+	/* 0xf0 to 0xfe: 4MB increments starting at 4MB */
+	if (gms < 0xf0)
+		return (size_t)gms * MB(32);
 	else
-		/* 4MB increments starting at 0xf0 for 4MB */
-		return (gmch_ctrl - 0xf0 + 1) << 22;
+		return (size_t)(gms - 0xf0 + 1) * MB(4);
 }
 
 typedef size_t (*stolen_size_fn)(int num, int slot, int func);
@@ -493,27 +467,27 @@ static const struct intel_stolen_funcs i865_stolen_funcs __initconst = {
 };
 
 static const struct intel_stolen_funcs gen3_stolen_funcs __initconst = {
-	.base = intel_stolen_base,
+	.base = gen3_stolen_base,
 	.size = gen3_stolen_size,
 };
 
 static const struct intel_stolen_funcs gen6_stolen_funcs __initconst = {
-	.base = intel_stolen_base,
+	.base = gen3_stolen_base,
 	.size = gen6_stolen_size,
 };
 
 static const struct intel_stolen_funcs gen8_stolen_funcs __initconst = {
-	.base = intel_stolen_base,
+	.base = gen3_stolen_base,
 	.size = gen8_stolen_size,
 };
 
 static const struct intel_stolen_funcs gen9_stolen_funcs __initconst = {
-	.base = intel_stolen_base,
+	.base = gen3_stolen_base,
 	.size = gen9_stolen_size,
 };
 
 static const struct intel_stolen_funcs chv_stolen_funcs __initconst = {
-	.base = intel_stolen_base,
+	.base = gen3_stolen_base,
 	.size = chv_stolen_size,
 };
 
@@ -554,7 +528,7 @@ static void __init intel_graphics_stolen(int num, int slot, int func)
 {
 	size_t size;
 	int i;
-	u32 start;
+	phys_addr_t start;
 	u16 device, subvendor, subdevice;
 
 	device = read_pci_config_16(num, slot, func, PCI_DEVICE_ID);
@@ -569,8 +543,8 @@ static void __init intel_graphics_stolen(int num, int slot, int func)
 			size = stolen_funcs->size(num, slot, func);
 			start = stolen_funcs->base(num, slot, func, size);
 			if (size && start) {
-				printk(KERN_INFO "Reserving Intel graphics stolen memory at 0x%x-0x%x\n",
-				       start, start + (u32)size - 1);
+				printk(KERN_INFO "Reserving Intel graphics stolen memory at 0x%llx-0x%llx\n",
+				       start, start + size - 1);
 				/* Mark this space as reserved */
 				e820_add_region(start, size, E820_RESERVED);
 				sanitize_e820_map(e820.map,
diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
index b7ce963..68fde8f 100644
--- a/drivers/gpu/drm/i915/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
@@ -109,9 +109,9 @@ static unsigned long i915_stolen_to_physical(struct drm_device *dev)
 	if (INTEL_INFO(dev)->gen >= 3) {
 		u32 bsm;
 
-		pci_read_config_dword(dev->pdev, BSM, &bsm);
+		pci_read_config_dword(dev->pdev, INTEL_BSM, &bsm);
 
-		base = bsm & BSM_MASK;
+		base = bsm & INTEL_BSM_MASK;
 	} else if (IS_I865G(dev)) {
 		u16 toud = 0;
 
diff --git a/include/drm/i915_drm.h b/include/drm/i915_drm.h
index 595f85c..b1755f8 100644
--- a/include/drm/i915_drm.h
+++ b/include/drm/i915_drm.h
@@ -92,4 +92,7 @@ extern bool i915_gpu_turbo_disable(void);
 #define    I845_TSEG_SIZE_512K	(2 << 1)
 #define    I845_TSEG_SIZE_1M	(3 << 1)
 
+#define INTEL_BSM 0x5c
+#define   INTEL_BSM_MASK (0xFFFF << 20)
+
 #endif				/* _I915_DRM_H_ */
-- 
2.5.5

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

[PATCH v2 2/2] drm/i915: Function per early graphics quirk

[Joonas Lahtinen]

Move graphics stolen memory related early quirk into a function to
allow easy adding of other graphics quirks to fix memory maps on
machines running old BIOS versions.

While at it;
- _funcs -> _ops to follow de facto naming
- make the iteration code tad more readable
- remove unused variables

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 arch/x86/kernel/early-quirks.c | 187 +++++++++++++++++++++--------------------
 1 file changed, 98 insertions(+), 89 deletions(-)

diff --git a/arch/x86/kernel/early-quirks.c b/arch/x86/kernel/early-quirks.c
index 70e0ab6..d2f75b4 100644
--- a/arch/x86/kernel/early-quirks.c
+++ b/arch/x86/kernel/early-quirks.c
@@ -423,11 +423,6 @@ static size_t __init chv_stolen_size(int num, int slot, int func)
 		return (size_t)(gms - 0x17 + 9) * MB(4);
 }
 
-struct intel_stolen_funcs {
-	size_t (*size)(int num, int slot, int func);
-	phys_addr_t (*base)(int num, int slot, int func, size_t size);
-};
-
 static size_t __init gen9_stolen_size(int num, int slot, int func)
 {
 	u16 gmch_ctrl;
@@ -444,115 +439,129 @@ static size_t __init gen9_stolen_size(int num, int slot, int func)
 		return (size_t)(gms - 0xf0 + 1) * MB(4);
 }
 
-typedef size_t (*stolen_size_fn)(int num, int slot, int func);
+struct intel_early_ops {
+	size_t (*stolen_size)(int num, int slot, int func);
+	phys_addr_t (*stolen_base)(int num, int slot, int func, size_t size);
+};
 
-static const struct intel_stolen_funcs i830_stolen_funcs __initconst = {
-	.base = i830_stolen_base,
-	.size = i830_stolen_size,
+static const struct intel_early_ops i830_early_ops __initconst = {
+	.stolen_base = i830_stolen_base,
+	.stolen_size = i830_stolen_size,
 };
 
-static const struct intel_stolen_funcs i845_stolen_funcs __initconst = {
-	.base = i845_stolen_base,
-	.size = i830_stolen_size,
+static const struct intel_early_ops i845_early_ops __initconst = {
+	.stolen_base = i845_stolen_base,
+	.stolen_size = i830_stolen_size,
 };
 
-static const struct intel_stolen_funcs i85x_stolen_funcs __initconst = {
-	.base = i85x_stolen_base,
-	.size = gen3_stolen_size,
+static const struct intel_early_ops i85x_early_ops __initconst = {
+	.stolen_base = i85x_stolen_base,
+	.stolen_size = gen3_stolen_size,
 };
 
-static const struct intel_stolen_funcs i865_stolen_funcs __initconst = {
-	.base = i865_stolen_base,
-	.size = gen3_stolen_size,
+static const struct intel_early_ops i865_early_ops __initconst = {
+	.stolen_base = i865_stolen_base,
+	.stolen_size = gen3_stolen_size,
 };
 
-static const struct intel_stolen_funcs gen3_stolen_funcs __initconst = {
-	.base = gen3_stolen_base,
-	.size = gen3_stolen_size,
+static const struct intel_early_ops gen3_early_ops __initconst = {
+	.stolen_base = gen3_stolen_base,
+	.stolen_size = gen3_stolen_size,
 };
 
-static const struct intel_stolen_funcs gen6_stolen_funcs __initconst = {
-	.base = gen3_stolen_base,
-	.size = gen6_stolen_size,
+static const struct intel_early_ops gen6_early_ops __initconst = {
+	.stolen_base = gen3_stolen_base,
+	.stolen_size = gen6_stolen_size,
 };
 
-static const struct intel_stolen_funcs gen8_stolen_funcs __initconst = {
-	.base = gen3_stolen_base,
-	.size = gen8_stolen_size,
+static const struct intel_early_ops gen8_early_ops __initconst = {
+	.stolen_base = gen3_stolen_base,
+	.stolen_size = gen8_stolen_size,
 };
 
-static const struct intel_stolen_funcs gen9_stolen_funcs __initconst = {
-	.base = gen3_stolen_base,
-	.size = gen9_stolen_size,
+static const struct intel_early_ops gen9_early_ops __initconst = {
+	.stolen_base = gen3_stolen_base,
+	.stolen_size = gen9_stolen_size,
 };
 
-static const struct intel_stolen_funcs chv_stolen_funcs __initconst = {
-	.base = gen3_stolen_base,
-	.size = chv_stolen_size,
+static const struct intel_early_ops chv_early_ops __initconst = {
+	.stolen_base = gen3_stolen_base,
+	.stolen_size = chv_stolen_size,
 };
 
-static const struct pci_device_id intel_stolen_ids[] __initconst = {
-	INTEL_I830_IDS(&i830_stolen_funcs),
-	INTEL_I845G_IDS(&i845_stolen_funcs),
-	INTEL_I85X_IDS(&i85x_stolen_funcs),
-	INTEL_I865G_IDS(&i865_stolen_funcs),
-	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),
-	INTEL_CHV_IDS(&chv_stolen_funcs),
-	INTEL_SKL_IDS(&gen9_stolen_funcs),
-	INTEL_BXT_IDS(&gen9_stolen_funcs),
-	INTEL_KBL_IDS(&gen9_stolen_funcs),
+static const struct pci_device_id intel_early_ids[] __initconst = {
+	INTEL_I830_IDS(&i830_early_ops),
+	INTEL_I845G_IDS(&i845_early_ops),
+	INTEL_I85X_IDS(&i85x_early_ops),
+	INTEL_I865G_IDS(&i865_early_ops),
+	INTEL_I915G_IDS(&gen3_early_ops),
+	INTEL_I915GM_IDS(&gen3_early_ops),
+	INTEL_I945G_IDS(&gen3_early_ops),
+	INTEL_I945GM_IDS(&gen3_early_ops),
+	INTEL_VLV_M_IDS(&gen6_early_ops),
+	INTEL_VLV_D_IDS(&gen6_early_ops),
+	INTEL_PINEVIEW_IDS(&gen3_early_ops),
+	INTEL_I965G_IDS(&gen3_early_ops),
+	INTEL_G33_IDS(&gen3_early_ops),
+	INTEL_I965GM_IDS(&gen3_early_ops),
+	INTEL_GM45_IDS(&gen3_early_ops),
+	INTEL_G45_IDS(&gen3_early_ops),
+	INTEL_IRONLAKE_D_IDS(&gen3_early_ops),
+	INTEL_IRONLAKE_M_IDS(&gen3_early_ops),
+	INTEL_SNB_D_IDS(&gen6_early_ops),
+	INTEL_SNB_M_IDS(&gen6_early_ops),
+	INTEL_IVB_M_IDS(&gen6_early_ops),
+	INTEL_IVB_D_IDS(&gen6_early_ops),
+	INTEL_HSW_D_IDS(&gen6_early_ops),
+	INTEL_HSW_M_IDS(&gen6_early_ops),
+	INTEL_BDW_M_IDS(&gen8_early_ops),
+	INTEL_BDW_D_IDS(&gen8_early_ops),
+	INTEL_CHV_IDS(&chv_early_ops),
+	INTEL_SKL_IDS(&gen9_early_ops),
+	INTEL_BXT_IDS(&gen9_early_ops),
+	INTEL_KBL_IDS(&gen9_early_ops),
 };
 
-static void __init intel_graphics_stolen(int num, int slot, int func)
+static void __init
+intel_graphics_stolen(int num, int slot, int func,
+		      const struct intel_early_ops *early_ops)
 {
+	phys_addr_t base;
 	size_t size;
+
+	size = early_ops->stolen_size(num, slot, func);
+	base = early_ops->stolen_base(num, slot, func, size);
+
+	if (!size || !base)
+		return;
+
+	printk(KERN_INFO "Reserving Intel graphics stolen memory at "
+	       "0x%llx-0x%llx\n", base, base + size - 1);
+
+	/* Mark this space as reserved */
+	e820_add_region(base, size, E820_RESERVED);
+	sanitize_e820_map(e820.map, ARRAY_SIZE(e820.map), &e820.nr_map);
+}
+
+static void __init intel_graphics_quirks(int num, int slot, int func)
+{
+	const struct intel_early_ops *early_ops;
+	u16 device;
 	int i;
-	phys_addr_t start;
-	u16 device, subvendor, subdevice;
 
 	device = read_pci_config_16(num, slot, func, PCI_DEVICE_ID);
-	subvendor = read_pci_config_16(num, slot, func,
-				       PCI_SUBSYSTEM_VENDOR_ID);
-	subdevice = read_pci_config_16(num, slot, func, PCI_SUBSYSTEM_ID);
-
-	for (i = 0; i < ARRAY_SIZE(intel_stolen_ids); i++) {
-		if (intel_stolen_ids[i].device == device) {
-			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) {
-				printk(KERN_INFO "Reserving Intel graphics stolen memory at 0x%llx-0x%llx\n",
-				       start, start + size - 1);
-				/* Mark this space as reserved */
-				e820_add_region(start, size, E820_RESERVED);
-				sanitize_e820_map(e820.map,
-						  ARRAY_SIZE(e820.map),
-						  &e820.nr_map);
-			}
-			return;
-		}
+
+	for (i = 0; i < ARRAY_SIZE(intel_early_ids); i++) {
+		kernel_ulong_t driver_data = intel_early_ids[i].driver_data;
+
+		if (intel_early_ids[i].device != device)
+			continue;
+
+		early_ops = (typeof(early_ops))driver_data;
+
+		intel_graphics_stolen(num, slot, func, early_ops);
+
+		return;
 	}
 }
 
@@ -601,7 +610,7 @@ static struct chipset early_qrk[] __initdata = {
 	{ PCI_VENDOR_ID_INTEL, 0x3406, PCI_CLASS_BRIDGE_HOST,
 	  PCI_BASE_CLASS_BRIDGE, 0, intel_remapping_check },
 	{ PCI_VENDOR_ID_INTEL, PCI_ANY_ID, PCI_CLASS_DISPLAY_VGA, PCI_ANY_ID,
-	  QFLAG_APPLY_ONCE, intel_graphics_stolen },
+	  QFLAG_APPLY_ONCE, intel_graphics_quirks },
 	/*
 	 * HPET on the current version of the Baytrail platform has accuracy
 	 * problems: it will halt in deep idle state - so we disable it.
-- 
2.5.5

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

Re: [PATCH v2 2/2] drm/i915: Function per early graphics quirk

[Joonas Lahtinen]

Hi,

Adding x86 maintainers as CC as they were missing from the original
patch due to some confusion in best practices.

This patch was merged to the drm-intel.git after reviewing and testing.

Regards, Joonas

On pe, 2016-04-22 at 15:28 +0300, Joonas Lahtinen wrote:
> Move graphics stolen memory related early quirk into a function to
> allow easy adding of other graphics quirks to fix memory maps on
> machines running old BIOS versions.
> 
> While at it;
> - _funcs -> _ops to follow de facto naming
> - make the iteration code tad more readable
> - remove unused variables
> 
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> ---
>  arch/x86/kernel/early-quirks.c | 187 +++++++++++++++++++++--------------------
>  1 file changed, 98 insertions(+), 89 deletions(-)
> 
> diff --git a/arch/x86/kernel/early-quirks.c b/arch/x86/kernel/early-quirks.c
> index 70e0ab6..d2f75b4 100644
> --- a/arch/x86/kernel/early-quirks.c
> +++ b/arch/x86/kernel/early-quirks.c
> @@ -423,11 +423,6 @@ static size_t __init chv_stolen_size(int num, int slot, int func)
>  		return (size_t)(gms - 0x17 + 9) * MB(4);
>  }
>  
> -struct intel_stolen_funcs {
> -	size_t (*size)(int num, int slot, int func);
> -	phys_addr_t (*base)(int num, int slot, int func, size_t size);
> -};
> -
>  static size_t __init gen9_stolen_size(int num, int slot, int func)
>  {
>  	u16 gmch_ctrl;
> @@ -444,115 +439,129 @@ static size_t __init gen9_stolen_size(int num, int slot, int func)
>  		return (size_t)(gms - 0xf0 + 1) * MB(4);
>  }
>  
> -typedef size_t (*stolen_size_fn)(int num, int slot, int func);
> +struct intel_early_ops {
> +	size_t (*stolen_size)(int num, int slot, int func);
> +	phys_addr_t (*stolen_base)(int num, int slot, int func, size_t size);
> +};
>  
> -static const struct intel_stolen_funcs i830_stolen_funcs __initconst = {
> -	.base = i830_stolen_base,
> -	.size = i830_stolen_size,
> +static const struct intel_early_ops i830_early_ops __initconst = {
> +	.stolen_base = i830_stolen_base,
> +	.stolen_size = i830_stolen_size,
>  };
>  
> -static const struct intel_stolen_funcs i845_stolen_funcs __initconst = {
> -	.base = i845_stolen_base,
> -	.size = i830_stolen_size,
> +static const struct intel_early_ops i845_early_ops __initconst = {
> +	.stolen_base = i845_stolen_base,
> +	.stolen_size = i830_stolen_size,
>  };
>  
> -static const struct intel_stolen_funcs i85x_stolen_funcs __initconst = {
> -	.base = i85x_stolen_base,
> -	.size = gen3_stolen_size,
> +static const struct intel_early_ops i85x_early_ops __initconst = {
> +	.stolen_base = i85x_stolen_base,
> +	.stolen_size = gen3_stolen_size,
>  };
>  
> -static const struct intel_stolen_funcs i865_stolen_funcs __initconst = {
> -	.base = i865_stolen_base,
> -	.size = gen3_stolen_size,
> +static const struct intel_early_ops i865_early_ops __initconst = {
> +	.stolen_base = i865_stolen_base,
> +	.stolen_size = gen3_stolen_size,
>  };
>  
> -static const struct intel_stolen_funcs gen3_stolen_funcs __initconst = {
> -	.base = gen3_stolen_base,
> -	.size = gen3_stolen_size,
> +static const struct intel_early_ops gen3_early_ops __initconst = {
> +	.stolen_base = gen3_stolen_base,
> +	.stolen_size = gen3_stolen_size,
>  };
>  
> -static const struct intel_stolen_funcs gen6_stolen_funcs __initconst = {
> -	.base = gen3_stolen_base,
> -	.size = gen6_stolen_size,
> +static const struct intel_early_ops gen6_early_ops __initconst = {
> +	.stolen_base = gen3_stolen_base,
> +	.stolen_size = gen6_stolen_size,
>  };
>  
> -static const struct intel_stolen_funcs gen8_stolen_funcs __initconst = {
> -	.base = gen3_stolen_base,
> -	.size = gen8_stolen_size,
> +static const struct intel_early_ops gen8_early_ops __initconst = {
> +	.stolen_base = gen3_stolen_base,
> +	.stolen_size = gen8_stolen_size,
>  };
>  
> -static const struct intel_stolen_funcs gen9_stolen_funcs __initconst = {
> -	.base = gen3_stolen_base,
> -	.size = gen9_stolen_size,
> +static const struct intel_early_ops gen9_early_ops __initconst = {
> +	.stolen_base = gen3_stolen_base,
> +	.stolen_size = gen9_stolen_size,
>  };
>  
> -static const struct intel_stolen_funcs chv_stolen_funcs __initconst = {
> -	.base = gen3_stolen_base,
> -	.size = chv_stolen_size,
> +static const struct intel_early_ops chv_early_ops __initconst = {
> +	.stolen_base = gen3_stolen_base,
> +	.stolen_size = chv_stolen_size,
>  };
>  
> -static const struct pci_device_id intel_stolen_ids[] __initconst = {
> -	INTEL_I830_IDS(&i830_stolen_funcs),
> -	INTEL_I845G_IDS(&i845_stolen_funcs),
> -	INTEL_I85X_IDS(&i85x_stolen_funcs),
> -	INTEL_I865G_IDS(&i865_stolen_funcs),
> -	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),
> -	INTEL_CHV_IDS(&chv_stolen_funcs),
> -	INTEL_SKL_IDS(&gen9_stolen_funcs),
> -	INTEL_BXT_IDS(&gen9_stolen_funcs),
> -	INTEL_KBL_IDS(&gen9_stolen_funcs),
> +static const struct pci_device_id intel_early_ids[] __initconst = {
> +	INTEL_I830_IDS(&i830_early_ops),
> +	INTEL_I845G_IDS(&i845_early_ops),
> +	INTEL_I85X_IDS(&i85x_early_ops),
> +	INTEL_I865G_IDS(&i865_early_ops),
> +	INTEL_I915G_IDS(&gen3_early_ops),
> +	INTEL_I915GM_IDS(&gen3_early_ops),
> +	INTEL_I945G_IDS(&gen3_early_ops),
> +	INTEL_I945GM_IDS(&gen3_early_ops),
> +	INTEL_VLV_M_IDS(&gen6_early_ops),
> +	INTEL_VLV_D_IDS(&gen6_early_ops),
> +	INTEL_PINEVIEW_IDS(&gen3_early_ops),
> +	INTEL_I965G_IDS(&gen3_early_ops),
> +	INTEL_G33_IDS(&gen3_early_ops),
> +	INTEL_I965GM_IDS(&gen3_early_ops),
> +	INTEL_GM45_IDS(&gen3_early_ops),
> +	INTEL_G45_IDS(&gen3_early_ops),
> +	INTEL_IRONLAKE_D_IDS(&gen3_early_ops),
> +	INTEL_IRONLAKE_M_IDS(&gen3_early_ops),
> +	INTEL_SNB_D_IDS(&gen6_early_ops),
> +	INTEL_SNB_M_IDS(&gen6_early_ops),
> +	INTEL_IVB_M_IDS(&gen6_early_ops),
> +	INTEL_IVB_D_IDS(&gen6_early_ops),
> +	INTEL_HSW_D_IDS(&gen6_early_ops),
> +	INTEL_HSW_M_IDS(&gen6_early_ops),
> +	INTEL_BDW_M_IDS(&gen8_early_ops),
> +	INTEL_BDW_D_IDS(&gen8_early_ops),
> +	INTEL_CHV_IDS(&chv_early_ops),
> +	INTEL_SKL_IDS(&gen9_early_ops),
> +	INTEL_BXT_IDS(&gen9_early_ops),
> +	INTEL_KBL_IDS(&gen9_early_ops),
>  };
>  
> -static void __init intel_graphics_stolen(int num, int slot, int func)
> +static void __init
> +intel_graphics_stolen(int num, int slot, int func,
> +		      const struct intel_early_ops *early_ops)
>  {
> +	phys_addr_t base;
>  	size_t size;
> +
> +	size = early_ops->stolen_size(num, slot, func);
> +	base = early_ops->stolen_base(num, slot, func, size);
> +
> +	if (!size || !base)
> +		return;
> +
> +	printk(KERN_INFO "Reserving Intel graphics stolen memory at "
> +	       "0x%llx-0x%llx\n", base, base + size - 1);
> +
> +	/* Mark this space as reserved */
> +	e820_add_region(base, size, E820_RESERVED);
> +	sanitize_e820_map(e820.map, ARRAY_SIZE(e820.map), &e820.nr_map);
> +}
> +
> +static void __init intel_graphics_quirks(int num, int slot, int func)
> +{
> +	const struct intel_early_ops *early_ops;
> +	u16 device;
>  	int i;
> -	phys_addr_t start;
> -	u16 device, subvendor, subdevice;
>  
>  	device = read_pci_config_16(num, slot, func, PCI_DEVICE_ID);
> -	subvendor = read_pci_config_16(num, slot, func,
> -				       PCI_SUBSYSTEM_VENDOR_ID);
> -	subdevice = read_pci_config_16(num, slot, func, PCI_SUBSYSTEM_ID);
> -
> -	for (i = 0; i < ARRAY_SIZE(intel_stolen_ids); i++) {
> -		if (intel_stolen_ids[i].device == device) {
> -			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) {
> -				printk(KERN_INFO "Reserving Intel graphics stolen memory at 0x%llx-0x%llx\n",
> -				       start, start + size - 1);
> -				/* Mark this space as reserved */
> -				e820_add_region(start, size, E820_RESERVED);
> -				sanitize_e820_map(e820.map,
> -						  ARRAY_SIZE(e820.map),
> -						  &e820.nr_map);
> -			}
> -			return;
> -		}
> +
> +	for (i = 0; i < ARRAY_SIZE(intel_early_ids); i++) {
> +		kernel_ulong_t driver_data = intel_early_ids[i].driver_data;
> +
> +		if (intel_early_ids[i].device != device)
> +			continue;
> +
> +		early_ops = (typeof(early_ops))driver_data;
> +
> +		intel_graphics_stolen(num, slot, func, early_ops);
> +
> +		return;
>  	}
>  }
>  
> @@ -601,7 +610,7 @@ static struct chipset early_qrk[] __initdata = {
>  	{ PCI_VENDOR_ID_INTEL, 0x3406, PCI_CLASS_BRIDGE_HOST,
>  	  PCI_BASE_CLASS_BRIDGE, 0, intel_remapping_check },
>  	{ PCI_VENDOR_ID_INTEL, PCI_ANY_ID, PCI_CLASS_DISPLAY_VGA, PCI_ANY_ID,
> -	  QFLAG_APPLY_ONCE, intel_graphics_stolen },
> +	  QFLAG_APPLY_ONCE, intel_graphics_quirks },
>  	/*
>  	 * HPET on the current version of the Baytrail platform has accuracy
>  	 * problems: it will halt in deep idle state - so we disable it.
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.BAT: warning for series starting with [1/2] drm/i915: Canonicalize stolen memory calculations (rev2)

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915: Canonicalize stolen memory calculations (rev2)
URL   : https://patchwork.freedesktop.org/series/6153/
State : warning

== Summary ==

Series 6153v2 Series without cover letter
http://patchwork.freedesktop.org/api/1.0/series/6153/revisions/2/mbox/

Test gem_busy:
        Subgroup basic-blt:
                pass       -> SKIP       (bsw-nuc-2)
Test kms_force_connector_basic:
        Subgroup force-edid:
                pass       -> SKIP       (ilk-hp8440p)
Test kms_pipe_crc_basic:
        Subgroup read-crc-pipe-b-frame-sequence:
                pass       -> SKIP       (skl-nuci5)

bdw-nuci7        total:193  pass:181  dwarn:0   dfail:0   fail:0   skip:12 
bdw-ultra        total:193  pass:170  dwarn:0   dfail:0   fail:0   skip:23 
bsw-nuc-2        total:192  pass:152  dwarn:0   dfail:0   fail:0   skip:40 
byt-nuc          total:192  pass:154  dwarn:0   dfail:0   fail:0   skip:38 
hsw-brixbox      total:193  pass:169  dwarn:0   dfail:0   fail:0   skip:24 
hsw-gt2          total:193  pass:174  dwarn:0   dfail:0   fail:0   skip:19 
ilk-hp8440p      total:193  pass:135  dwarn:0   dfail:0   fail:0   skip:58 
ivb-t430s        total:193  pass:165  dwarn:0   dfail:0   fail:0   skip:28 
skl-i7k-2        total:193  pass:168  dwarn:0   dfail:0   fail:0   skip:25 
skl-nuci5        total:193  pass:181  dwarn:0   dfail:0   fail:0   skip:12 
snb-dellxps      total:193  pass:155  dwarn:0   dfail:0   fail:0   skip:38 
snb-x220t        total:193  pass:155  dwarn:0   dfail:0   fail:1   skip:37 

Results at /archive/results/CI_IGT_test/Patchwork_1994/

111261dcf565dd98389a1fd4353c6099712e9f81 drm-intel-nightly: 2016y-04m-22d-12h-33m-25s UTC integration manifest
d8163cb drm/i915: Function per early graphics quirk
9202502 drm/i915: Canonicalize stolen memory calculations

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

✗ Fi.CI.BAT: warning for series starting with [1/2] drm/i915: Canonicalize stolen memory calculations (rev2)

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915: Canonicalize stolen memory calculations (rev2)
URL   : https://patchwork.freedesktop.org/series/6153/
State : warning

== Summary ==

Series 6153v2 Series without cover letter
http://patchwork.freedesktop.org/api/1.0/series/6153/revisions/2/mbox/

Test kms_pipe_crc_basic:
        Subgroup hang-read-crc-pipe-a:
                pass       -> DMESG-WARN (snb-dellxps)

bdw-ultra        total:193  pass:170  dwarn:0   dfail:0   fail:0   skip:23 
bsw-nuc-2        total:192  pass:153  dwarn:0   dfail:0   fail:0   skip:39 
byt-nuc          total:192  pass:154  dwarn:0   dfail:0   fail:0   skip:38 
ilk-hp8440p      total:193  pass:136  dwarn:0   dfail:0   fail:0   skip:57 
ivb-t430s        total:193  pass:165  dwarn:0   dfail:0   fail:0   skip:28 
skl-i7k-2        total:193  pass:168  dwarn:0   dfail:0   fail:0   skip:25 
skl-nuci5        total:193  pass:182  dwarn:0   dfail:0   fail:0   skip:11 
snb-dellxps      total:193  pass:154  dwarn:1   dfail:0   fail:0   skip:38 

Results at /archive/results/CI_IGT_test/Patchwork_2031/

340c485ad98d0ec0369a3b18d4a09938f3f5537d drm-intel-nightly: 2016y-04m-22d-17h-32m-25s UTC integration manifest
f9b4352 drm/i915: Function per early graphics quirk
135a1d0 drm/i915: Canonicalize stolen memory calculations

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

Re: ✗ Fi.CI.BAT: warning for series starting with [1/2] drm/i915: Canonicalize stolen memory calculations (rev2)

[Joonas Lahtinen]

On su, 2016-04-24 at 07:53 +0000, Patchwork wrote:
> == Series Details ==
> 
> Series: series starting with [1/2] drm/i915: Canonicalize stolen memory calculations (rev2)
> URL   : https://patchwork.freedesktop.org/series/6153/
> State : warning
> 
> == Summary ==
> 
> Series 6153v2 Series without cover letter
> http://patchwork.freedesktop.org/api/1.0/series/6153/revisions/2/mbox/
> 
> Test kms_pipe_crc_basic:
>         Subgroup hang-read-crc-pipe-a:
>                 pass       -> DMESG-WARN (snb-dellxps)
> 

This related to atomic stuff, opened a new bug for it:

https://bugs.freedesktop.org/show_bug.cgi?id=95125

Would be nice to hear an ACK from Mika/Tvrtko before committing.

Regards, Joonas

> bdw-ultra        total:193  pass:170  dwarn:0   dfail:0   fail:0   skip:23 
> bsw-nuc-2        total:192  pass:153  dwarn:0   dfail:0   fail:0   skip:39 
> byt-nuc          total:192  pass:154  dwarn:0   dfail:0   fail:0   skip:38 
> ilk-hp8440p      total:193  pass:136  dwarn:0   dfail:0   fail:0   skip:57 
> ivb-t430s        total:193  pass:165  dwarn:0   dfail:0   fail:0   skip:28 
> skl-i7k-2        total:193  pass:168  dwarn:0   dfail:0   fail:0   skip:25 
> skl-nuci5        total:193  pass:182  dwarn:0   dfail:0   fail:0   skip:11 
> snb-dellxps      total:193  pass:154  dwarn:1   dfail:0   fail:0   skip:38 
> 
> Results at /archive/results/CI_IGT_test/Patchwork_2031/
> 
> 340c485ad98d0ec0369a3b18d4a09938f3f5537d drm-intel-nightly: 2016y-04m-22d-17h-32m-25s UTC integration manifest
> f9b4352 drm/i915: Function per early graphics quirk
> 135a1d0 drm/i915: Canonicalize stolen memory calculations
> 
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: ✗ Fi.CI.BAT: warning for series starting with [1/2] drm/i915: Canonicalize stolen memory calculations (rev2)

[Tvrtko Ursulin]

On 25/04/16 09:54, Joonas Lahtinen wrote:
> On su, 2016-04-24 at 07:53 +0000, Patchwork wrote:
>> == Series Details ==
>>
>> Series: series starting with [1/2] drm/i915: Canonicalize stolen memory calculations (rev2)
>> URL   : https://patchwork.freedesktop.org/series/6153/
>> State : warning
>>
>> == Summary ==
>>
>> Series 6153v2 Series without cover letter
>> http://patchwork.freedesktop.org/api/1.0/series/6153/revisions/2/mbox/
>>
>> Test kms_pipe_crc_basic:
>>          Subgroup hang-read-crc-pipe-a:
>>                  pass       -> DMESG-WARN (snb-dellxps)
>>
>
> This related to atomic stuff, opened a new bug for it:
>
> https://bugs.freedesktop.org/show_bug.cgi?id=95125
>
> Would be nice to hear an ACK from Mika/Tvrtko before committing.

Without going into details covered by the r-b already, the high level 
cleanup looks fine to me, so:

Acked-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

Tvrtko

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

Re: ✗ Fi.CI.BAT: warning for series starting with [1/2] drm/i915: Canonicalize stolen memory calculations (rev2)

[Joonas Lahtinen]

On ma, 2016-04-25 at 10:23 +0100, Tvrtko Ursulin wrote:
> Acked-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Merged, thanks for the R-b and A-b!

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation

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

Re: ✗ Fi.CI.BAT: warning for series starting with [1/2] drm/i915: Canonicalize stolen memory calculations (rev2)

[Mika Kuoppala]

Joonas Lahtinen <joonas.lahtinen@linux.intel.com> writes:

> [ text/plain ]
> On ma, 2016-04-25 at 10:23 +0100, Tvrtko Ursulin wrote:
>> Acked-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
> Merged, thanks for the R-b and A-b!
>

Too late in this party, but read it through and it
clears things up.

> Regards, Joonas
> -- 
> Joonas Lahtinen
> Open Source Technology Center
> Intel Corporation
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx