* ville.syrjala@xxxxxxxxxxxxxxx <ville.syrjala@xxxxxxxxxxxxxxx> wrote:
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>
> There doesn't seem to an explicit stolen memory base register on gen2.
> Some old comment in the i915 code suggests we should get it via
> max_low_pfn_mapped, but that's clearly a bad idea on my MGM.
>
> The e820 map in said machine looks like this:
> [ 0.000000] BIOS-e820: [mem 0x0000000000000000-0x000000000009f7ff] usable
> [ 0.000000] BIOS-e820: [mem 0x000000000009f800-0x000000000009ffff] reserved
> [ 0.000000] BIOS-e820: [mem 0x00000000000ce000-0x00000000000cffff] reserved
> [ 0.000000] BIOS-e820: [mem 0x00000000000dc000-0x00000000000fffff] reserved
> [ 0.000000] BIOS-e820: [mem 0x0000000000100000-0x000000001f6effff] usable
> [ 0.000000] BIOS-e820: [mem 0x000000001f6f0000-0x000000001f6f7fff] ACPI data
> [ 0.000000] BIOS-e820: [mem 0x000000001f6f8000-0x000000001f6fffff] ACPI NVS
> [ 0.000000] BIOS-e820: [mem 0x000000001f700000-0x000000001fffffff] reserved
> [ 0.000000] BIOS-e820: [mem 0x00000000fec10000-0x00000000fec1ffff] reserved
> [ 0.000000] BIOS-e820: [mem 0x00000000ffb00000-0x00000000ffbfffff] reserved
> [ 0.000000] BIOS-e820: [mem 0x00000000fff00000-0x00000000ffffffff] reserved
>
> That makes max_low_pfn_mapped = 1f6f0000, so assuming our stolen memory
> would start there would place it on top of some ACPI memory regions.
> So not a good idea as already stated.
>
> The 9MB region after the ACPI regions at 0x1f700000 however looks
> promising given that the macine reports the stolen memory size to be
> 8MB. Looking at the PGTBL_CTL register, the GTT entries are at offset
> 0x1fee00000, and given that the GTT entries occupy 128KB, it looks like
> the stolen memory could start at 0x1f700000 and the GTT entries would
> occupy the last 128KB of the stolen memory. I have no idea about the
> extra 1MB after the GTT entries.
>
> I tested this on the machine in question, and so far I've not seen any
> issues with the use the this memory region. Hopefully the same rules
> hold for all gen2 machines...
>
> Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> Cc: Ingo Molnar <mingo@xxxxxxxxxx>
> Cc: "H. Peter Anvin" <hpa@xxxxxxxxx>
> Cc: x86@xxxxxxxxxx
> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> ---
> arch/x86/kernel/early-quirks.c | 70 ++++++++++++++++++++++++++++++++++++++++++
> include/drm/i915_drm.h | 12 ++++++++
> 2 files changed, 82 insertions(+)
>
> diff --git a/arch/x86/kernel/early-quirks.c b/arch/x86/kernel/early-quirks.c
> index ca49966..6cd90b4 100644
> --- a/arch/x86/kernel/early-quirks.c
> +++ b/arch/x86/kernel/early-quirks.c
> @@ -247,6 +247,62 @@ static u32 __init intel_stolen_base(int num, int slot, int func, size_t size)
> #define MB(x) (KB (KB (x)))
> #define GB(x) (MB (KB (x)))
>
> +/*
> + * Gen2 stolen base is tricky. Try to deduce it from the
> + * page table base address.
> + */
> +static u32 __init gen2_stolen_base(int num, int slot, int func, size_t size)
> +{
> + void __iomem *regs;
> + u32 reg_addr;
> + u32 pgtbl_ctl;
> +
> + reg_addr = read_pci_config(0, 2, 0, I810_MMADDR) & 0xfff80000;
Why is the mmaddr rounded to 512K? Do the lower 19 bits hold some
other piece of information, or are they always zero?
> + regs = early_ioremap(reg_addr, KB(64));
> + if (!regs)
> + return 0;
Emitting a warning here might be useful, if anyone runs into this.
> + pgtbl_ctl = readl(regs + I810_PGTBL_CTL);
> + early_iounmap(regs, KB(64));
> +
> + /* GTT disabled? */
> + if (!(pgtbl_ctl & I810_PGTBL_ENABLED))
> + return 0;
> +
> + pgtbl_ctl &= I810_PGTBL_ADDRESS_MASK;
> +
> + /* Assume GTT entries occupy the last 128KB of stolen/local memory */
> + return pgtbl_ctl + KB(128) - size;
Btw., I love the cleanliness of the KB() / MB() / GB() macros, it
ought to move from agp.h to include/linux/kernel.h or so.
> +static size_t __init i830_stolen_size(int num, int slot, int func)
> +{
> + size_t stolen_size;
> + u16 gmch_ctrl;
> +
> + gmch_ctrl = read_pci_config_16(0, 0, 0, I830_GMCH_CTRL);
> +
> + switch (gmch_ctrl & I830_GMCH_GMS_MASK) {
> + case I830_GMCH_GMS_STOLEN_512:
> + stolen_size = KB(512);
> + break;
> + case I830_GMCH_GMS_STOLEN_1024:
> + stolen_size = MB(1);
> + break;
> + case I830_GMCH_GMS_STOLEN_8192:
> + stolen_size = MB(8);
> + break;
> + case I830_GMCH_GMS_LOCAL:
> + /* local memory isn't part of the normal address space */
> + stolen_size = 0;
> + break;
> + default:
> + return 0;
> + }
> +
> + return stolen_size;
Nit: looks like the labels 'I830_GMCH_GMS_LOCAL' and 'default' have
the same effect so they could be merged.
> diff --git a/include/drm/i915_drm.h b/include/drm/i915_drm.h
> index 97d5497..1526546 100644
> --- a/include/drm/i915_drm.h
> +++ b/include/drm/i915_drm.h
> @@ -54,8 +54,16 @@ extern bool i915_gpu_turbo_disable(void);
> #define BDW_GMCH_GMS_SHIFT 8
> #define BDW_GMCH_GMS_MASK 0xff
>
> +#define I810_MMADDR 0x14
> +
> #define I830_GMCH_CTRL 0x52
Nit: the I810_MMADDR define appears to be out of alignment with the
surrounding constants, is that intentional?
Thanks,
Ingo
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
