[Intel-gfx] [PATCH 02/18] drm/i915: Fix detection of stolen base for gen2
Ben Widawsky
ben at bwidawsk.net
Fri Nov 2 00:51:02 CET 2012
On Fri, 19 Oct 2012 18:03:08 +0100
Chris Wilson <chris at chris-wilson.co.uk> wrote:
> It was not until the G33 refresh, that a PCI config register was
> introduced that explicitly said where the stolen memory was. Prior to
> 865G there was not even a register that said where the end of usable
> low memory was and where the stolen memory began (or ended depending
> upon chipset). Before then, one has to look at the BIOS memory maps to
> find the Top of Memory. Alas that is not exported by arch/x86 and so we
> have to resort to disabling stolen memory on gen2 for the time being.
>
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 1 +
> drivers/gpu/drm/i915/i915_gem_stolen.c | 69 ++++++++++++++------------------
> 2 files changed, 31 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 4728d30..687f379 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -705,6 +705,7 @@ typedef struct drm_i915_private {
> unsigned long gtt_start;
> unsigned long gtt_mappable_end;
> unsigned long gtt_end;
> + unsigned long stolen_base; /* limited to low memory (32-bit) */
>
> struct io_mapping *gtt_mapping;
> phys_addr_t gtt_base_addr;
> diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
> index ada2e90..a01ff74 100644
> --- a/drivers/gpu/drm/i915/i915_gem_stolen.c
> +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
> @@ -43,56 +43,43 @@
> * for is a boon.
> */
>
> -#define PTE_ADDRESS_MASK 0xfffff000
> -#define PTE_ADDRESS_MASK_HIGH 0x000000f0 /* i915+ */
> -#define PTE_MAPPING_TYPE_UNCACHED (0 << 1)
> -#define PTE_MAPPING_TYPE_DCACHE (1 << 1) /* i830 only */
> -#define PTE_MAPPING_TYPE_CACHED (3 << 1)
> -#define PTE_MAPPING_TYPE_MASK (3 << 1)
> -#define PTE_VALID (1 << 0)
> -
> -/**
> - * i915_stolen_to_phys - take an offset into stolen memory and turn it into
> - * a physical one
> - * @dev: drm device
> - * @offset: address to translate
> - *
> - * Some chip functions require allocations from stolen space and need the
> - * physical address of the memory in question.
> - */
> -static unsigned long i915_stolen_to_phys(struct drm_device *dev, u32 offset)
> +static unsigned long i915_stolen_to_physical(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct pci_dev *pdev = dev_priv->bridge_dev;
> u32 base;
>
> -#if 0
> /* On the machines I have tested the Graphics Base of Stolen Memory
> - * is unreliable, so compute the base by subtracting the stolen memory
> - * from the Top of Low Usable DRAM which is where the BIOS places
> - * the graphics stolen memory.
> + * is unreliable, so on those compute the base by subtracting the
> + * stolen memory from the Top of Low Usable DRAM which is where the
> + * BIOS places the graphics stolen memory.
> + *
> + * On gen2, the layout is slightly different with the Graphics Segment
> + * immediately following Top of Memory (or Top of Usable DRAM). Note
> + * it appears that TOUD is only reported by 865g, so we just use the
> + * top of memory as determined by the e820 probe.
> + *
> + * XXX gen2 requires an unavailable symbol and 945gm fails with
> + * its value of TOLUD.
> */
> + base = 0;
> if (INTEL_INFO(dev)->gen > 3 || IS_G33(dev)) {
> - /* top 32bits are reserved = 0 */
> + /* Read Graphics Base of Stolen Memory directly */
> pci_read_config_dword(pdev, 0xA4, &base);
> - } else {
> - /* XXX presume 8xx is the same as i915 */
> - pci_bus_read_config_dword(pdev->bus, 2, 0x5C, &base);
> - }
> -#else
> - if (INTEL_INFO(dev)->gen > 3 || IS_G33(dev)) {
> - u16 val;
> - pci_read_config_word(pdev, 0xb0, &val);
> - base = val >> 4 << 20;
> - } else {
> +#if 0
> + } else if (IS_GEN3(dev)) {
> u8 val;
> + /* Stolen is immediately below Top of Low Usable DRAM */
> pci_read_config_byte(pdev, 0x9c, &val);
> base = val >> 3 << 27;
> - }
> - base -= dev_priv->mm.gtt->stolen_size;
> + base -= dev_priv->mm.gtt->stolen_size;
> + } else {
> + /* Stolen is immediately above Top of Memory */
> + base = max_low_pfn_mapped << PAGE_SHIFT;
> #endif
> + }
>
> - return base + offset;
> + return base;
> }
>
Comments on this below.
> static void i915_warn_stolen(struct drm_device *dev)
> @@ -117,7 +104,7 @@ static void i915_setup_compression(struct drm_device *dev, int size)
> if (!compressed_fb)
> goto err;
>
> - cfb_base = i915_stolen_to_phys(dev, compressed_fb->start);
> + cfb_base = dev_priv->mm.stolen_base + compressed_fb->start;
> if (!cfb_base)
> goto err_fb;
>
> @@ -130,7 +117,7 @@ static void i915_setup_compression(struct drm_device *dev, int size)
> if (!compressed_llb)
> goto err_fb;
>
> - ll_base = i915_stolen_to_phys(dev, compressed_llb->start);
> + ll_base = dev_priv->mm.stolen_base + compressed_llb->start;
> if (!ll_base)
> goto err_llb;
> }
> @@ -149,7 +136,7 @@ static void i915_setup_compression(struct drm_device *dev, int size)
> }
>
> DRM_DEBUG_KMS("FBC base 0x%08lx, ll base 0x%08lx, size %dM\n",
> - cfb_base, ll_base, size >> 20);
> + (long)cfb_base, (long)ll_base, size >> 20);
> return;
>
> err_llb:
> @@ -181,6 +168,10 @@ int i915_gem_init_stolen(struct drm_device *dev)
> struct drm_i915_private *dev_priv = dev->dev_private;
> unsigned long prealloc_size = dev_priv->mm.gtt->stolen_size;
>
> + dev_priv->mm.stolen_base = i915_stolen_to_physical(dev);
> + if (dev_priv->mm.stolen_base == 0)
> + return 0;
> +
> /* Basic memrange allocator for stolen space */
> drm_mm_init(&dev_priv->mm.stolen, 0, prealloc_size);
>
Since I found viewing the diff difficult for this patch, I am going to
write the before and after (with the #if 0 blocks removed):
Before:
if (INTEL_INFO(dev)->gen > 3 || IS_G33(dev)) {
u16 val;
pci_read_config_word(pdev, 0xb0, &val);
base = val >> 4 << 20;
} else {
u8 val;
pci_read_config_byte(pdev, 0x9c, &val);
base = val >> 3 << 27;
}
base -= dev_priv->mm.gtt->stolen_size;
After:
base = 0;
if (INTEL_INFO(dev)->gen > 3 || IS_G33(dev)) {
/* Read Graphics Base of Stolen Memory directly */
pci_read_config_dword(pdev, 0xA4, &base);
I like that this is a simple helper now to calculate GSM Base. However,
I am completely baffled by this patch. The read of 0xA4 did exist
before, but was #if 0'd out appears to be the PCI capabilities pointer
as of GEN6 (I won't check before that). The before code on the other
hand at least looks correct for gen6 (BDSM - GGMS).
This seems like it will definitely break SNB+, which would be bad for
bisection.
Has git done something funny here?
--
Ben Widawsky, Intel Open Source Technology Center
More information about the Intel-gfx
mailing list