[PATCH] Revert "drm/i915: Calculate correct stolen size for GEN7+"

[PATCH] Revert "drm/i915: Calculate correct stolen size for GEN7+"

[Ben Widawsky]

This reverts commit 03752f5b7b77b95d83479885040950fba1250850.

This revert requires a bit of explanation on how I understand things
work. Internally the architects/designers decide how the stolen encoding
works. We put it in a doc. BIOS writers take these docs and implement
it. Driver writers read the doc too, and read the value left by the BIOS
writers, and then we make magic.

The failing here is that in the docs we had[1] contained two different
definitions for this register for Gen7. (We have both a PCI register,
and an MMIO, and each of these were different). At the time [2] of
03752f5, we asked the architects what the correct value should be; but
that doesn't match the reality (BIOS) unfortunately.

So on all machines I can get my hands on, this revert is the right thing
to do. I've also worked with the product group to confirm that they
agree this revert is what we should do. People using HW made my "people"
who both write their own BIOS, and have access to our docs (Apple?)

[1] The docs are still wrong on this one. Now instead of two registers with
two definitions, we have one register with BOTH definitions, progress?
[2] The open source PRMs have the "wrong" definitions in chapter Volume
1 part6, section 1.1.12.

This digging was inspired by Paulo.

Conflicts:
	drivers/gpu/drm/i915/i915_gem_gtt.c

Cc: Paulo Zanoni <przanoni@gmail.com>
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 15 +--------------
 drivers/gpu/drm/i915/i915_reg.h     |  2 --
 2 files changed, 1 insertion(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 37058ff2..de6e7c5 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -743,15 +743,6 @@ static inline size_t gen6_get_stolen_size(u16 snb_gmch_ctl)
 	return snb_gmch_ctl << 25; /* 32 MB units */
 }
 
-static inline size_t gen7_get_stolen_size(u16 snb_gmch_ctl)
-{
-	static const int stolen_decoder[] = {
-		0, 0, 0, 0, 0, 32, 48, 64, 128, 256, 96, 160, 224, 352};
-	snb_gmch_ctl >>= IVB_GMCH_GMS_SHIFT;
-	snb_gmch_ctl &= IVB_GMCH_GMS_MASK;
-	return stolen_decoder[snb_gmch_ctl] << 20;
-}
-
 static int gen6_gmch_probe(struct drm_device *dev,
 			   size_t *gtt_total,
 			   size_t *stolen,
@@ -781,11 +772,7 @@ static int gen6_gmch_probe(struct drm_device *dev,
 	pci_read_config_word(dev->pdev, SNB_GMCH_CTRL, &snb_gmch_ctl);
 	gtt_size = gen6_get_total_gtt_size(snb_gmch_ctl);
 
-	if (IS_GEN7(dev) && !IS_VALLEYVIEW(dev))
-		*stolen = gen7_get_stolen_size(snb_gmch_ctl);
-	else
-		*stolen = gen6_get_stolen_size(snb_gmch_ctl);
-
+	*stolen = gen6_get_stolen_size(snb_gmch_ctl);
 	*gtt_total = (gtt_size / sizeof(gen6_gtt_pte_t)) << PAGE_SHIFT;
 
 	/* For Modern GENs the PTEs and register space are split in the BAR */
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 95ae5cf..f2a7a8c 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -46,8 +46,6 @@
 #define    SNB_GMCH_GGMS_MASK	0x3
 #define    SNB_GMCH_GMS_SHIFT   3 /* Graphics Mode Select */
 #define    SNB_GMCH_GMS_MASK    0x1f
-#define    IVB_GMCH_GMS_SHIFT   4
-#define    IVB_GMCH_GMS_MASK    0xf
 
 
 /* PCI config space */
-- 
1.8.2.2

Re: [PATCH] Revert "drm/i915: Calculate correct stolen size for GEN7+"

[Ben Widawsky]

On Wed, May 01, 2013 at 11:00:34AM -0700, Ben Widawsky wrote:
> This reverts commit 03752f5b7b77b95d83479885040950fba1250850.
> 
> This revert requires a bit of explanation on how I understand things
> work. Internally the architects/designers decide how the stolen encoding
> works. We put it in a doc. BIOS writers take these docs and implement
> it. Driver writers read the doc too, and read the value left by the BIOS
> writers, and then we make magic.
> 
> The failing here is that in the docs we had[1] contained two different
> definitions for this register for Gen7. (We have both a PCI register,
> and an MMIO, and each of these were different). At the time [2] of
> 03752f5, we asked the architects what the correct value should be; but
> that doesn't match the reality (BIOS) unfortunately.
> 
> So on all machines I can get my hands on, this revert is the right thing
> to do. I've also worked with the product group to confirm that they
> agree this revert is what we should do. People using HW made my "people"
> who both write their own BIOS, and have access to our docs (Apple?)

Ooops, left this bit out... These "people" may not agree with the
revert. I don't have machines to test.

> 
> [1] The docs are still wrong on this one. Now instead of two registers with
> two definitions, we have one register with BOTH definitions, progress?
> [2] The open source PRMs have the "wrong" definitions in chapter Volume
> 1 part6, section 1.1.12.
> 
> This digging was inspired by Paulo.
> 
> Conflicts:
> 	drivers/gpu/drm/i915/i915_gem_gtt.c
> 
> Cc: Paulo Zanoni <przanoni@gmail.com>
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> ---
>  drivers/gpu/drm/i915/i915_gem_gtt.c | 15 +--------------
>  drivers/gpu/drm/i915/i915_reg.h     |  2 --
>  2 files changed, 1 insertion(+), 16 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 37058ff2..de6e7c5 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -743,15 +743,6 @@ static inline size_t gen6_get_stolen_size(u16 snb_gmch_ctl)
>  	return snb_gmch_ctl << 25; /* 32 MB units */
>  }
>  
> -static inline size_t gen7_get_stolen_size(u16 snb_gmch_ctl)
> -{
> -	static const int stolen_decoder[] = {
> -		0, 0, 0, 0, 0, 32, 48, 64, 128, 256, 96, 160, 224, 352};
> -	snb_gmch_ctl >>= IVB_GMCH_GMS_SHIFT;
> -	snb_gmch_ctl &= IVB_GMCH_GMS_MASK;
> -	return stolen_decoder[snb_gmch_ctl] << 20;
> -}
> -
>  static int gen6_gmch_probe(struct drm_device *dev,
>  			   size_t *gtt_total,
>  			   size_t *stolen,
> @@ -781,11 +772,7 @@ static int gen6_gmch_probe(struct drm_device *dev,
>  	pci_read_config_word(dev->pdev, SNB_GMCH_CTRL, &snb_gmch_ctl);
>  	gtt_size = gen6_get_total_gtt_size(snb_gmch_ctl);
>  
> -	if (IS_GEN7(dev) && !IS_VALLEYVIEW(dev))
> -		*stolen = gen7_get_stolen_size(snb_gmch_ctl);
> -	else
> -		*stolen = gen6_get_stolen_size(snb_gmch_ctl);
> -
> +	*stolen = gen6_get_stolen_size(snb_gmch_ctl);
>  	*gtt_total = (gtt_size / sizeof(gen6_gtt_pte_t)) << PAGE_SHIFT;
>  
>  	/* For Modern GENs the PTEs and register space are split in the BAR */
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 95ae5cf..f2a7a8c 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -46,8 +46,6 @@
>  #define    SNB_GMCH_GGMS_MASK	0x3
>  #define    SNB_GMCH_GMS_SHIFT   3 /* Graphics Mode Select */
>  #define    SNB_GMCH_GMS_MASK    0x1f
> -#define    IVB_GMCH_GMS_SHIFT   4
> -#define    IVB_GMCH_GMS_MASK    0xf
>  
>  
>  /* PCI config space */
> -- 
> 1.8.2.2
> 

-- 
Ben Widawsky, Intel Open Source Technology Center

Re: [PATCH] Revert "drm/i915: Calculate correct stolen size for GEN7+"

[Daniel Vetter]

On Wed, May 01, 2013 at 11:03:10AM -0700, Ben Widawsky wrote:
> On Wed, May 01, 2013 at 11:00:34AM -0700, Ben Widawsky wrote:
> > This reverts commit 03752f5b7b77b95d83479885040950fba1250850.
> > 
> > This revert requires a bit of explanation on how I understand things
> > work. Internally the architects/designers decide how the stolen encoding
> > works. We put it in a doc. BIOS writers take these docs and implement
> > it. Driver writers read the doc too, and read the value left by the BIOS
> > writers, and then we make magic.
> > 
> > The failing here is that in the docs we had[1] contained two different
> > definitions for this register for Gen7. (We have both a PCI register,
> > and an MMIO, and each of these were different). At the time [2] of
> > 03752f5, we asked the architects what the correct value should be; but
> > that doesn't match the reality (BIOS) unfortunately.
> > 
> > So on all machines I can get my hands on, this revert is the right thing
> > to do. I've also worked with the product group to confirm that they
> > agree this revert is what we should do. People using HW made my "people"
> > who both write their own BIOS, and have access to our docs (Apple?)
> 
> Ooops, left this bit out... These "people" may not agree with the
> revert. I don't have machines to test.
> 
> > 
> > [1] The docs are still wrong on this one. Now instead of two registers with
> > two definitions, we have one register with BOTH definitions, progress?
> > [2] The open source PRMs have the "wrong" definitions in chapter Volume
> > 1 part6, section 1.1.12.
> > 
> > This digging was inspired by Paulo.
> > 
> > Conflicts:
> > 	drivers/gpu/drm/i915/i915_gem_gtt.c
> > 
> > Cc: Paulo Zanoni <przanoni@gmail.com>
> > Signed-off-by: Ben Widawsky <ben@bwidawsk.net>

Picked up for -fixes, thanks for the patch.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch