diff mbox

[1/6] drm/i915: export the stolen region as a resource

Message ID 20171122211920.20993-2-matthew.auld@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Matthew Auld Nov. 22, 2017, 9:19 p.m. UTC
We duplicate the stolen discovery code in early-quirks and in i915,
however if we just export the region as a resource from early-quirks we
can nuke the duplication.

Suggested-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Matthew Auld <matthew.auld@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: dri-devel@lists.freedesktop.org
Cc: x86@kernel.org
---
 arch/x86/kernel/early-quirks.c         |   6 ++
 drivers/gpu/drm/i915/i915_gem_gtt.c    |  51 +--------------
 drivers/gpu/drm/i915/i915_gem_stolen.c | 109 +--------------------------------
 include/drm/i915_drm.h                 |   3 +
 4 files changed, 13 insertions(+), 156 deletions(-)

Comments

Ingo Molnar Nov. 23, 2017, 6:17 a.m. UTC | #1
* Matthew Auld <matthew.auld@intel.com> wrote:

> We duplicate the stolen discovery code in early-quirks and in i915,
> however if we just export the region as a resource from early-quirks we
> can nuke the duplication.
> 
> Suggested-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: H. Peter Anvin <hpa@zytor.com>
> Cc: dri-devel@lists.freedesktop.org
> Cc: x86@kernel.org
> ---
>  arch/x86/kernel/early-quirks.c         |   6 ++
>  drivers/gpu/drm/i915/i915_gem_gtt.c    |  51 +--------------
>  drivers/gpu/drm/i915/i915_gem_stolen.c | 109 +--------------------------------
>  include/drm/i915_drm.h                 |   3 +
>  4 files changed, 13 insertions(+), 156 deletions(-)

If it's truly identical:

  Acked-by: Ingo Molnar <mingo@kernel.org>

Thanks,

	Ingo
Joonas Lahtinen Nov. 23, 2017, 7:48 a.m. UTC | #2
+ Dave as a heads-up

On Thu, 2017-11-23 at 07:17 +0100, Ingo Molnar wrote:
> * Matthew Auld <matthew.auld@intel.com> wrote:
> 
> > We duplicate the stolen discovery code in early-quirks and in i915,
> > however if we just export the region as a resource from early-quirks we
> > can nuke the duplication.
> > 
> > Suggested-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Signed-off-by: Matthew Auld <matthew.auld@intel.com>
> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > Cc: Ingo Molnar <mingo@kernel.org>
> > Cc: H. Peter Anvin <hpa@zytor.com>
> > Cc: dri-devel@lists.freedesktop.org
> > Cc: x86@kernel.org
> > ---
> >  arch/x86/kernel/early-quirks.c         |   6 ++
> >  drivers/gpu/drm/i915/i915_gem_gtt.c    |  51 +--------------
> >  drivers/gpu/drm/i915/i915_gem_stolen.c | 109 +--------------------------------
> >  include/drm/i915_drm.h                 |   3 +
> >  4 files changed, 13 insertions(+), 156 deletions(-)
> 
> If it's truly identical:
> 
>   Acked-by: Ingo Molnar <mingo@kernel.org>

Are you fine with us merging this together with the rest of the series
through the DRM tree once it's all reviewed?

That'd help not requiring so many backmerges.

Regards, Joonas
Ingo Molnar Nov. 23, 2017, 7:59 a.m. UTC | #3
* Joonas Lahtinen <joonas.lahtinen@linux.intel.com> wrote:

> + Dave as a heads-up
> 
> On Thu, 2017-11-23 at 07:17 +0100, Ingo Molnar wrote:
> > * Matthew Auld <matthew.auld@intel.com> wrote:
> > 
> > > We duplicate the stolen discovery code in early-quirks and in i915,
> > > however if we just export the region as a resource from early-quirks we
> > > can nuke the duplication.
> > > 
> > > Suggested-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > > Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > Signed-off-by: Matthew Auld <matthew.auld@intel.com>
> > > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > > Cc: Ingo Molnar <mingo@kernel.org>
> > > Cc: H. Peter Anvin <hpa@zytor.com>
> > > Cc: dri-devel@lists.freedesktop.org
> > > Cc: x86@kernel.org
> > > ---
> > >  arch/x86/kernel/early-quirks.c         |   6 ++
> > >  drivers/gpu/drm/i915/i915_gem_gtt.c    |  51 +--------------
> > >  drivers/gpu/drm/i915/i915_gem_stolen.c | 109 +--------------------------------
> > >  include/drm/i915_drm.h                 |   3 +
> > >  4 files changed, 13 insertions(+), 156 deletions(-)
> > 
> > If it's truly identical:
> > 
> >   Acked-by: Ingo Molnar <mingo@kernel.org>
> 
> Are you fine with us merging this together with the rest of the series
> through the DRM tree once it's all reviewed?
> 
> That'd help not requiring so many backmerges.

Yes, sure, feel free - that's also where most of the relevant testing is done for 
the affected hardware.

Thanks,

	Ingo
Joonas Lahtinen Nov. 24, 2017, 7:52 a.m. UTC | #4
+ Ville to comment if the removed code loses some meaningful comments
or not. I already went through the code doing consolidations, about a
year ago, so I may be blind to it.

On Wed, 2017-11-22 at 21:19 +0000, Matthew Auld wrote:
> We duplicate the stolen discovery code in early-quirks and in i915,
> however if we just export the region as a resource from early-quirks we
> can nuke the duplication.
> 
> Suggested-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: H. Peter Anvin <hpa@zytor.com>
> Cc: dri-devel@lists.freedesktop.org
> Cc: x86@kernel.org

<SNIP>

> @@ -548,6 +551,9 @@ intel_graphics_stolen(int num, int slot, int func,
>  	printk(KERN_INFO "Reserving Intel graphics memory at %pa-%pa\n",
>  	       &base, &end);
>  
> +	intel_graphics_stolen_res.start = base;
> +	intel_graphics_stolen_res.end = end;

You can take advantage of the newly establisted resource by using %pR
in the printk.

I sent a patch to convert the function signatures to resource_size_t
for a less painful future.

Maybe squash just the early quirks/header change portion of this patch
to that patch, then we can iterate on the i915 changes on a reminder of
the series on top of that. 

Regards, Joonas
diff mbox

Patch

diff --git a/arch/x86/kernel/early-quirks.c b/arch/x86/kernel/early-quirks.c
index 1e82f787c160..a98a51cd15a7 100644
--- a/arch/x86/kernel/early-quirks.c
+++ b/arch/x86/kernel/early-quirks.c
@@ -531,6 +531,9 @@  static const struct pci_device_id intel_early_ids[] __initconst = {
 	INTEL_CNL_IDS(&gen9_early_ops),
 };
 
+struct resource intel_graphics_stolen_res = DEFINE_RES_MEM(0, 0);
+EXPORT_SYMBOL(intel_graphics_stolen_res);
+
 static void __init
 intel_graphics_stolen(int num, int slot, int func,
 		      const struct intel_early_ops *early_ops)
@@ -548,6 +551,9 @@  intel_graphics_stolen(int num, int slot, int func,
 	printk(KERN_INFO "Reserving Intel graphics memory at %pa-%pa\n",
 	       &base, &end);
 
+	intel_graphics_stolen_res.start = base;
+	intel_graphics_stolen_res.end = end;
+
 	/* Mark this space as reserved */
 	e820__range_add(base, size, E820_TYPE_RESERVED);
 	e820__update_table(e820_table);
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index e101b9a98957..cccd0cb51e09 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -2949,50 +2949,6 @@  static unsigned int chv_get_total_gtt_size(u16 gmch_ctrl)
 	return 0;
 }
 
-static size_t gen6_get_stolen_size(u16 snb_gmch_ctl)
-{
-	snb_gmch_ctl >>= SNB_GMCH_GMS_SHIFT;
-	snb_gmch_ctl &= SNB_GMCH_GMS_MASK;
-	return (size_t)snb_gmch_ctl << 25; /* 32 MB units */
-}
-
-static size_t gen8_get_stolen_size(u16 bdw_gmch_ctl)
-{
-	bdw_gmch_ctl >>= BDW_GMCH_GMS_SHIFT;
-	bdw_gmch_ctl &= BDW_GMCH_GMS_MASK;
-	return (size_t)bdw_gmch_ctl << 25; /* 32 MB units */
-}
-
-static size_t chv_get_stolen_size(u16 gmch_ctrl)
-{
-	gmch_ctrl >>= SNB_GMCH_GMS_SHIFT;
-	gmch_ctrl &= 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 (size_t)gmch_ctrl << 25;
-	else if (gmch_ctrl < 0x17)
-		return (size_t)(gmch_ctrl - 0x11 + 2) << 22;
-	else
-		return (size_t)(gmch_ctrl - 0x17 + 9) << 22;
-}
-
-static size_t gen9_get_stolen_size(u16 gen9_gmch_ctl)
-{
-	gen9_gmch_ctl >>= BDW_GMCH_GMS_SHIFT;
-	gen9_gmch_ctl &= BDW_GMCH_GMS_MASK;
-
-	if (gen9_gmch_ctl < 0xf0)
-		return (size_t)gen9_gmch_ctl << 25; /* 32 MB units */
-	else
-		/* 4MB increments starting at 0xf0 for 4MB */
-		return (size_t)(gen9_gmch_ctl - 0xf0 + 1) << 22;
-}
-
 static int ggtt_probe_common(struct i915_ggtt *ggtt, u64 size)
 {
 	struct drm_i915_private *dev_priv = ggtt->base.i915;
@@ -3343,14 +3299,13 @@  static int gen8_gmch_probe(struct i915_ggtt *ggtt)
 
 	pci_read_config_word(pdev, SNB_GMCH_CTRL, &snb_gmch_ctl);
 
+	ggtt->stolen_size = resource_size(&intel_graphics_stolen_res);
+
 	if (INTEL_GEN(dev_priv) >= 9) {
-		ggtt->stolen_size = gen9_get_stolen_size(snb_gmch_ctl);
 		size = gen8_get_total_gtt_size(snb_gmch_ctl);
 	} else if (IS_CHERRYVIEW(dev_priv)) {
-		ggtt->stolen_size = chv_get_stolen_size(snb_gmch_ctl);
 		size = chv_get_total_gtt_size(snb_gmch_ctl);
 	} else {
-		ggtt->stolen_size = gen8_get_stolen_size(snb_gmch_ctl);
 		size = gen8_get_total_gtt_size(snb_gmch_ctl);
 	}
 
@@ -3408,7 +3363,7 @@  static int gen6_gmch_probe(struct i915_ggtt *ggtt)
 		DRM_ERROR("Can't set DMA mask/consistent mask (%d)\n", err);
 	pci_read_config_word(pdev, SNB_GMCH_CTRL, &snb_gmch_ctl);
 
-	ggtt->stolen_size = gen6_get_stolen_size(snb_gmch_ctl);
+	ggtt->stolen_size = resource_size(&intel_graphics_stolen_res);
 
 	size = gen6_get_total_gtt_size(snb_gmch_ctl);
 	ggtt->base.total = (size / sizeof(gen6_pte_t)) << PAGE_SHIFT;
diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
index 1877ae9a1d9b..f1b8eeda0058 100644
--- a/drivers/gpu/drm/i915/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
@@ -30,9 +30,6 @@ 
 #include <drm/i915_drm.h>
 #include "i915_drv.h"
 
-#define KB(x) ((x) * 1024)
-#define MB(x) (KB(x) * 1024)
-
 /*
  * The BIOS typically reserves some of the system's memory for the exclusive
  * use of the integrated graphics. This memory is no longer available for
@@ -81,113 +78,9 @@  void i915_gem_stolen_remove_node(struct drm_i915_private *dev_priv,
 
 static dma_addr_t i915_stolen_to_dma(struct drm_i915_private *dev_priv)
 {
-	struct pci_dev *pdev = dev_priv->drm.pdev;
 	struct i915_ggtt *ggtt = &dev_priv->ggtt;
+	dma_addr_t base = intel_graphics_stolen_res.start;
 	struct resource *r;
-	dma_addr_t base;
-
-	/* 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.
-	 *
-	 * On 865 we just check the TOUD register.
-	 *
-	 * 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.
-	 *
-	 */
-	base = 0;
-	if (INTEL_GEN(dev_priv) >= 3) {
-		u32 bsm;
-
-		pci_read_config_dword(pdev, INTEL_BSM, &bsm);
-
-		base = bsm & INTEL_BSM_MASK;
-	} else if (IS_I865G(dev_priv)) {
-		u32 tseg_size = 0;
-		u16 toud = 0;
-		u8 tmp;
-
-		pci_bus_read_config_byte(pdev->bus, PCI_DEVFN(0, 0),
-					 I845_ESMRAMC, &tmp);
-
-		if (tmp & TSEG_ENABLE) {
-			switch (tmp & I845_TSEG_SIZE_MASK) {
-			case I845_TSEG_SIZE_512K:
-				tseg_size = KB(512);
-				break;
-			case I845_TSEG_SIZE_1M:
-				tseg_size = MB(1);
-				break;
-			}
-		}
-
-		pci_bus_read_config_word(pdev->bus, PCI_DEVFN(0, 0),
-					 I865_TOUD, &toud);
-
-		base = (toud << 16) + tseg_size;
-	} else if (IS_I85X(dev_priv)) {
-		u32 tseg_size = 0;
-		u32 tom;
-		u8 tmp;
-
-		pci_bus_read_config_byte(pdev->bus, PCI_DEVFN(0, 0),
-					 I85X_ESMRAMC, &tmp);
-
-		if (tmp & TSEG_ENABLE)
-			tseg_size = MB(1);
-
-		pci_bus_read_config_byte(pdev->bus, PCI_DEVFN(0, 1),
-					 I85X_DRB3, &tmp);
-		tom = tmp * MB(32);
-
-		base = tom - tseg_size - ggtt->stolen_size;
-	} else if (IS_I845G(dev_priv)) {
-		u32 tseg_size = 0;
-		u32 tom;
-		u8 tmp;
-
-		pci_bus_read_config_byte(pdev->bus, PCI_DEVFN(0, 0),
-					 I845_ESMRAMC, &tmp);
-
-		if (tmp & TSEG_ENABLE) {
-			switch (tmp & I845_TSEG_SIZE_MASK) {
-			case I845_TSEG_SIZE_512K:
-				tseg_size = KB(512);
-				break;
-			case I845_TSEG_SIZE_1M:
-				tseg_size = MB(1);
-				break;
-			}
-		}
-
-		pci_bus_read_config_byte(pdev->bus, PCI_DEVFN(0, 0),
-					 I830_DRB3, &tmp);
-		tom = tmp * MB(32);
-
-		base = tom - tseg_size - ggtt->stolen_size;
-	} else if (IS_I830(dev_priv)) {
-		u32 tseg_size = 0;
-		u32 tom;
-		u8 tmp;
-
-		pci_bus_read_config_byte(pdev->bus, PCI_DEVFN(0, 0),
-					 I830_ESMRAMC, &tmp);
-
-		if (tmp & TSEG_ENABLE) {
-			if (tmp & I830_TSEG_SIZE_1M)
-				tseg_size = MB(1);
-			else
-				tseg_size = KB(512);
-		}
-
-		pci_bus_read_config_byte(pdev->bus, PCI_DEVFN(0, 0),
-					 I830_DRB3, &tmp);
-		tom = tmp * MB(32);
-
-		base = tom - tseg_size - ggtt->stolen_size;
-	}
 
 	if (base == 0 || add_overflows(base, ggtt->stolen_size))
 		return 0;
diff --git a/include/drm/i915_drm.h b/include/drm/i915_drm.h
index 4e1b274e1164..c9e5a6621b95 100644
--- a/include/drm/i915_drm.h
+++ b/include/drm/i915_drm.h
@@ -36,6 +36,9 @@  extern bool i915_gpu_lower(void);
 extern bool i915_gpu_busy(void);
 extern bool i915_gpu_turbo_disable(void);
 
+/* Exported from arch/x86/kernel/early-quirks.c */
+extern struct resource intel_graphics_stolen_res;
+
 /*
  * The Bridge device's PCI config space has information about the
  * fb aperture size and the amount of pre-reserved memory.