diff mbox

drm/i915: Account for TSEG size when determining 865G stolen base

Message ID 1470653919-27251-1-git-send-email-ville.syrjala@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ville Syrjala Aug. 8, 2016, 10:58 a.m. UTC 
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Looks like the TSEG lives just above TOUD, stolen comes after TSEG.

The spec seems somewhat self-contradictory in places, in the ESMRAMC
register desctription it says:
 TSEG Size:
  10=(TOUD + 512 KB) to TOUD
  11 =(TOUD + 1 MB) to TOUD

so that agrees with TSEG being at TOUD. But the example given
elsehwere in the spec says:

 TOUD equals 62.5 MB = 03E7FFFFh
 TSEG selected as 512 KB in size,
 Graphics local memory selected as 1 MB in size
 General System RAM available in system = 62.5 MB
 General system RAM range00000000h to 03E7FFFFh
 TSEG address range03F80000h to 03FFFFFFh
 TSEG pre-allocated from03F80000h to 03FFFFFFh
 Graphics local memory pre-allocated from03E80000h to 03F7FFFFh

so here we have TSEG above stolen.

Real world evidence agrees with the TOUD->TSEG->stolen order however, so
let's fix up the code to account for the TSEG size.

Cc: Taketo Kabe <fdporg@vega.pgw.jp>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: x86@kernel.org
Cc: stable@vger.kernel.org
Fixes: 0ad98c74e093 ("drm/i915: Determine the stolen memory base address on gen2")
Fixes: a4dff76924fe ("x86/gpu: Add Intel graphics stolen memory quirk for gen2 platforms")
Reported-by: Taketo Kabe <fdporg@vega.pgw.jp>
Tested-by: Taketo Kabe <fdporg@vega.pgw.jp>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96473
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 arch/x86/kernel/early-quirks.c         |  9 ++-------
 drivers/gpu/drm/i915/i915_gem_stolen.c | 23 +++++++++++++++++------
 2 files changed, 19 insertions(+), 13 deletions(-)

Comments

Ville Syrjala Aug. 9, 2016, 5:44 a.m. UTC | #1 
On Mon, Aug 08, 2016 at 11:25:22AM -0000, Patchwork wrote:
> == Series Details ==
> 
> Series: drm/i915: Account for TSEG size when determining 865G stolen base
> URL   : https://patchwork.freedesktop.org/series/10779/
> State : failure
> 
> == Summary ==
> 
> Series 10779v1 drm/i915: Account for TSEG size when determining 865G stolen base
> http://patchwork.freedesktop.org/api/1.0/series/10779/revisions/1/mbox
> 
> Test drv_module_reload_basic:
>                 pass       -> SKIP       (fi-skl-i5-6260u)

rmmod: ERROR: Module i915 is in use
rmmod: ERROR: Module i915 is in use
rmmod: ERROR: Module i915 is in use
rmmod: ERROR: Module i915 is in use
rmmod: ERROR: Module i915 is in use

same old stuff

> Test kms_cursor_legacy:
>         Subgroup basic-cursor-vs-flip-legacy:
>                 pass       -> FAIL       (ro-ilk1-i5-650)

(kms_cursor_legacy:8066) DEBUG: Test requirement passed: target > 1
(kms_cursor_legacy:8066) DEBUG: Using a target of 64 cursor updates per half-vblank
(kms_cursor_legacy:8066) WARNING: page flip 2 was delayed, missed 1 frames
(kms_cursor_legacy:8066) CRITICAL: Test assertion failure function basic_cursor_vs_flip, file kms_cursor_legacy.c:670:
(kms_cursor_legacy:8066) CRITICAL: Failed assertion: vbl.sequence == vblank_start + 60
(kms_cursor_legacy:8066) CRITICAL: error: 11071 != 11070

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

> 
> fi-hsw-i7-4770k  total:107  pass:91   dwarn:0   dfail:0   fail:0   skip:15 
> fi-kbl-qkkr      total:107  pass:83   dwarn:1   dfail:0   fail:0   skip:22 
> fi-skl-i5-6260u  total:107  pass:97   dwarn:0   dfail:0   fail:0   skip:9  
> fi-skl-i7-6700k  total:107  pass:84   dwarn:0   dfail:0   fail:0   skip:22 
> fi-snb-i7-2600   total:107  pass:77   dwarn:0   dfail:0   fail:0   skip:29 
> ro-bdw-i5-5250u  total:107  pass:97   dwarn:0   dfail:0   fail:0   skip:9  
> ro-bdw-i7-5557U  total:107  pass:97   dwarn:0   dfail:0   fail:0   skip:9  
> ro-bdw-i7-5600u  total:94   pass:69   dwarn:0   dfail:0   fail:0   skip:24 
> ro-bsw-n3050     total:240  pass:194  dwarn:0   dfail:0   fail:4   skip:42 
> ro-byt-n2820     total:240  pass:197  dwarn:0   dfail:0   fail:3   skip:40 
> ro-hsw-i3-4010u  total:107  pass:87   dwarn:0   dfail:0   fail:0   skip:19 
> ro-ilk1-i5-650   total:235  pass:172  dwarn:0   dfail:0   fail:3   skip:60 
> ro-ivb-i7-3770   total:107  pass:80   dwarn:0   dfail:0   fail:0   skip:26 
> ro-skl3-i5-6260u total:107  pass:98   dwarn:0   dfail:0   fail:0   skip:8  
> ro-bdw-i7-5600u failed to connect after reboot
> ro-hsw-i7-4770r failed to connect after reboot
> ro-ivb2-i7-3770 failed to connect after reboot
> 
> Results at /archive/results/CI_IGT_test/RO_Patchwork_1761/
> 
> 8ca71ca drm-intel-nightly: 2016y-08m-08d-09h-02m-24s UTC integration manifest
> 32bc6d7 drm/i915: Account for TSEG size when determining 865G stolen base
Chris Wilson Aug. 9, 2016, 8:53 a.m. UTC | #2 
On Mon, Aug 08, 2016 at 01:58:39PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Looks like the TSEG lives just above TOUD, stolen comes after TSEG.
> 
> The spec seems somewhat self-contradictory in places, in the ESMRAMC
> register desctription it says:
>  TSEG Size:
>   10=(TOUD + 512 KB) to TOUD
>   11 =(TOUD + 1 MB) to TOUD
> 
> so that agrees with TSEG being at TOUD. But the example given
> elsehwere in the spec says:
> 
>  TOUD equals 62.5 MB = 03E7FFFFh
>  TSEG selected as 512 KB in size,
>  Graphics local memory selected as 1 MB in size
>  General System RAM available in system = 62.5 MB
>  General system RAM range00000000h to 03E7FFFFh
>  TSEG address range03F80000h to 03FFFFFFh
>  TSEG pre-allocated from03F80000h to 03FFFFFFh
>  Graphics local memory pre-allocated from03E80000h to 03F7FFFFh

Found that example:

"""
Notes on Pre-Allocated Memory for Graphics

These register bits control the use of memory from main memory space as
graphics local memory.  The memory for TSEG is pre-allocated first and
then the graphics local memory is pre-allocated. 
"""
 
> so here we have TSEG above stolen.
> 
> Real world evidence agrees with the TOUD->TSEG->stolen order however, so
> let's fix up the code to account for the TSEG size.
> 
> Cc: Taketo Kabe <fdporg@vega.pgw.jp>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> Cc: x86@kernel.org
> Cc: stable@vger.kernel.org
> Fixes: 0ad98c74e093 ("drm/i915: Determine the stolen memory base address on gen2")
> Fixes: a4dff76924fe ("x86/gpu: Add Intel graphics stolen memory quirk for gen2 platforms")
> Reported-by: Taketo Kabe <fdporg@vega.pgw.jp>
> Tested-by: Taketo Kabe <fdporg@vega.pgw.jp>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96473
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Link: http://download.intel.com/design/chipsets/datashts/25251405.pdf
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
Ville Syrjala Aug. 11, 2016, 4:38 p.m. UTC | #3 
On Tue, Aug 09, 2016 at 09:53:47AM +0100, Chris Wilson wrote:
> On Mon, Aug 08, 2016 at 01:58:39PM +0300, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Looks like the TSEG lives just above TOUD, stolen comes after TSEG.
> > 
> > The spec seems somewhat self-contradictory in places, in the ESMRAMC
> > register desctription it says:
> >  TSEG Size:
> >   10=(TOUD + 512 KB) to TOUD
> >   11 =(TOUD + 1 MB) to TOUD
> > 
> > so that agrees with TSEG being at TOUD. But the example given
> > elsehwere in the spec says:
> > 
> >  TOUD equals 62.5 MB = 03E7FFFFh
> >  TSEG selected as 512 KB in size,
> >  Graphics local memory selected as 1 MB in size
> >  General System RAM available in system = 62.5 MB
> >  General system RAM range00000000h to 03E7FFFFh
> >  TSEG address range03F80000h to 03FFFFFFh
> >  TSEG pre-allocated from03F80000h to 03FFFFFFh
> >  Graphics local memory pre-allocated from03E80000h to 03F7FFFFh
> 
> Found that example:
> 
> """
> Notes on Pre-Allocated Memory for Graphics
> 
> These register bits control the use of memory from main memory space as
> graphics local memory.  The memory for TSEG is pre-allocated first and
> then the graphics local memory is pre-allocated. 
> """
>  
> > so here we have TSEG above stolen.
> > 
> > Real world evidence agrees with the TOUD->TSEG->stolen order however, so
> > let's fix up the code to account for the TSEG size.
> > 
> > Cc: Taketo Kabe <fdporg@vega.pgw.jp>
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Cc: Thomas Gleixner <tglx@linutronix.de>
> > Cc: Ingo Molnar <mingo@redhat.com>
> > Cc: "H. Peter Anvin" <hpa@zytor.com>
> > Cc: x86@kernel.org
> > Cc: stable@vger.kernel.org
> > Fixes: 0ad98c74e093 ("drm/i915: Determine the stolen memory base address on gen2")
> > Fixes: a4dff76924fe ("x86/gpu: Add Intel graphics stolen memory quirk for gen2 platforms")
> > Reported-by: Taketo Kabe <fdporg@vega.pgw.jp>
> > Tested-by: Taketo Kabe <fdporg@vega.pgw.jp>
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96473
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Link: http://download.intel.com/design/chipsets/datashts/25251405.pdf
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

Didn't see any objections from x86 folks, so I went and pushed this to
dinq. Thanks for the review.
diff mbox

Patch

diff --git a/arch/x86/kernel/early-quirks.c b/arch/x86/kernel/early-quirks.c
index de7501edb21c..8b8852bc2f4a 100644
--- a/arch/x86/kernel/early-quirks.c
+++ b/arch/x86/kernel/early-quirks.c
@@ -317,16 +317,11 @@  static phys_addr_t __init i85x_stolen_base(int num, int slot, int func,
 static phys_addr_t __init i865_stolen_base(int num, int slot, int func,
 					   size_t stolen_size)
 {
-	u16 toud;
+	u16 toud = 0;
 
-	/*
-	 * FIXME is the graphics stolen memory region
-	 * always at TOUD? Ie. is it always the last
-	 * one to be allocated by the BIOS?
-	 */
 	toud = read_pci_config_16(0, 0, 0, I865_TOUD);
 
-	return (phys_addr_t)toud << 16;
+	return (phys_addr_t)(toud << 16) + i845_tseg_size();
 }
 
 static phys_addr_t __init gen3_stolen_base(int num, int slot, int func,
diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
index 310756c30723..62e1a439023e 100644
--- a/drivers/gpu/drm/i915/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
@@ -115,17 +115,28 @@  static unsigned long i915_stolen_to_physical(struct drm_device *dev)
 
 		base = bsm & INTEL_BSM_MASK;
 	} else if (IS_I865G(dev)) {
+		u32 tseg_size = 0;
 		u16 toud = 0;
+		u8 tmp;
+
+		pci_bus_read_config_byte(dev->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;
+			}
+		}
 
-		/*
-		 * FIXME is the graphics stolen memory region
-		 * always at TOUD? Ie. is it always the last
-		 * one to be allocated by the BIOS?
-		 */
 		pci_bus_read_config_word(dev->pdev->bus, PCI_DEVFN(0, 0),
 					 I865_TOUD, &toud);
 
-		base = toud << 16;
+		base = (toud << 16) + tseg_size;
 	} else if (IS_I85X(dev)) {
 		u32 tseg_size = 0;
 		u32 tom;