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

[Ben Widawsky <ben@bwidawsk.net>]

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