* Ugly patches for stolen reservation
@ 2013-07-25 16:37 Jesse Barnes
2013-07-25 16:37 ` [PATCH 1/2] drm/i915: split PCI IDs out into i915_drm.h v3 Jesse Barnes
` (2 more replies)
0 siblings, 3 replies; 20+ messages in thread
From: Jesse Barnes @ 2013-07-25 16:37 UTC (permalink / raw)
To: intel-gfx, linux-kernel; +Cc: hpa, mingo, tglx, torvalds
Patch 2/2 has the description, but suffice it to say I'm not really
pleased with this, though it does solve a problem we have. On some
machines, we get MMIO space allocated on top of this hidden memory,
which can cause problems. I'm not sure if there are similar problems
for other hunks of the address space; if so it's possible this could be
made more general (though the bits for looking up the address of this
region are definitely Intel graphics specific).
Chris has some patches on top to add a new E820 type so we can look up
the region later, which removes some redundant code in the i915 driver
at least.
Any comments? I assume no one likes this, but maybe it's just another
early quirk we'll have to live with...
Thanks,
Jesse
^ permalink raw reply [flat|nested] 20+ messages in thread* [PATCH 1/2] drm/i915: split PCI IDs out into i915_drm.h v3 2013-07-25 16:37 Ugly patches for stolen reservation Jesse Barnes @ 2013-07-25 16:37 ` Jesse Barnes 2013-07-25 23:06 ` Chris Wilson 2013-07-25 16:37 ` [PATCH 2/2] x86: add early quirk for reserving Intel graphics stolen memory v3 Jesse Barnes 2013-07-25 20:05 ` Ugly patches for stolen reservation Ingo Molnar 2 siblings, 1 reply; 20+ messages in thread From: Jesse Barnes @ 2013-07-25 16:37 UTC (permalink / raw) To: intel-gfx, linux-kernel; +Cc: hpa, mingo, tglx, torvalds For use by userspace (at some point in the future) and other kernel code. v2: move PCI IDs to uabi (Chris) move PCI IDs to drm/ (Dave) v3: fixup Quanta detection - needs to come first (Daniel) Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> --- drivers/gpu/drm/i915/i915_drv.c | 164 +++++++----------------------- include/drm/i915_drm.h | 2 + include/drm/i915_pciids.h | 208 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 244 insertions(+), 130 deletions(-) create mode 100644 include/drm/i915_pciids.h diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index b07362f..e87bccf 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -140,25 +140,6 @@ MODULE_PARM_DESC(fastboot, "Try to skip unnecessary mode sets at boot time " static struct drm_driver driver; extern int intel_agp_enabled; -#define INTEL_VGA_DEVICE(id, info) { \ - .class = PCI_BASE_CLASS_DISPLAY << 16, \ - .class_mask = 0xff0000, \ - .vendor = 0x8086, \ - .device = id, \ - .subvendor = PCI_ANY_ID, \ - .subdevice = PCI_ANY_ID, \ - .driver_data = (unsigned long) info } - -#define INTEL_QUANTA_VGA_DEVICE(info) { \ - .class = PCI_BASE_CLASS_DISPLAY << 16, \ - .class_mask = 0xff0000, \ - .vendor = 0x8086, \ - .device = 0x16a, \ - .subvendor = 0x152d, \ - .subdevice = 0x8990, \ - .driver_data = (unsigned long) info } - - static const struct intel_device_info intel_i830_info = { .gen = 2, .is_mobile = 1, .cursor_needs_physical = 1, .num_pipes = 2, .has_overlay = 1, .overlay_needs_physical = 1, @@ -333,118 +314,41 @@ static const struct intel_device_info intel_haswell_m_info = { .has_vebox_ring = 1, }; +/* + * Make sure any device matches here are from most specific to most + * general. For example, since the Quanta match is based on the subsystem + * and subvendor IDs, we need it to come before the more general IVB + * PCI ID matches, otherwise we'll use the wrong info struct above. + */ +#define INTEL_PCI_IDS \ + INTEL_I830_IDS(&intel_i830_info), \ + INTEL_I845G_IDS(&intel_845g_info), \ + INTEL_I85X_IDS(&intel_i85x_info), \ + INTEL_I865G_IDS(&intel_i865g_info), \ + INTEL_I915G_IDS(&intel_i915g_info), \ + INTEL_I915GM_IDS(&intel_i915gm_info), \ + INTEL_I945G_IDS(&intel_i945g_info), \ + INTEL_I945GM_IDS(&intel_i945gm_info), \ + INTEL_I965G_IDS(&intel_i965g_info), \ + INTEL_G33_IDS(&intel_g33_info), \ + INTEL_I965GM_IDS(&intel_i965gm_info), \ + INTEL_GM45_IDS(&intel_gm45_info), \ + INTEL_G45_IDS(&intel_g45_info), \ + INTEL_PINEVIEW_IDS(&intel_pineview_info), \ + INTEL_IRONLAKE_D_IDS(&intel_ironlake_d_info), \ + INTEL_IRONLAKE_M_IDS(&intel_ironlake_m_info), \ + INTEL_SNB_D_IDS(&intel_sandybridge_d_info), \ + INTEL_SNB_M_IDS(&intel_sandybridge_m_info), \ + INTEL_IVB_Q_IDS(&intel_ivybridge_q_info), /* must be first IVB */ \ + INTEL_IVB_M_IDS(&intel_ivybridge_m_info), \ + INTEL_IVB_D_IDS(&intel_ivybridge_d_info), \ + INTEL_HSW_D_IDS(&intel_haswell_d_info), \ + INTEL_HSW_M_IDS(&intel_haswell_m_info), \ + INTEL_VLV_M_IDS(&intel_valleyview_m_info), \ + INTEL_VLV_D_IDS(&intel_valleyview_d_info) + static const struct pci_device_id pciidlist[] = { /* aka */ - INTEL_VGA_DEVICE(0x3577, &intel_i830_info), /* I830_M */ - INTEL_VGA_DEVICE(0x2562, &intel_845g_info), /* 845_G */ - INTEL_VGA_DEVICE(0x3582, &intel_i85x_info), /* I855_GM */ - INTEL_VGA_DEVICE(0x358e, &intel_i85x_info), - INTEL_VGA_DEVICE(0x2572, &intel_i865g_info), /* I865_G */ - INTEL_VGA_DEVICE(0x2582, &intel_i915g_info), /* I915_G */ - INTEL_VGA_DEVICE(0x258a, &intel_i915g_info), /* E7221_G */ - INTEL_VGA_DEVICE(0x2592, &intel_i915gm_info), /* I915_GM */ - INTEL_VGA_DEVICE(0x2772, &intel_i945g_info), /* I945_G */ - INTEL_VGA_DEVICE(0x27a2, &intel_i945gm_info), /* I945_GM */ - INTEL_VGA_DEVICE(0x27ae, &intel_i945gm_info), /* I945_GME */ - INTEL_VGA_DEVICE(0x2972, &intel_i965g_info), /* I946_GZ */ - INTEL_VGA_DEVICE(0x2982, &intel_i965g_info), /* G35_G */ - INTEL_VGA_DEVICE(0x2992, &intel_i965g_info), /* I965_Q */ - INTEL_VGA_DEVICE(0x29a2, &intel_i965g_info), /* I965_G */ - INTEL_VGA_DEVICE(0x29b2, &intel_g33_info), /* Q35_G */ - INTEL_VGA_DEVICE(0x29c2, &intel_g33_info), /* G33_G */ - INTEL_VGA_DEVICE(0x29d2, &intel_g33_info), /* Q33_G */ - INTEL_VGA_DEVICE(0x2a02, &intel_i965gm_info), /* I965_GM */ - INTEL_VGA_DEVICE(0x2a12, &intel_i965gm_info), /* I965_GME */ - INTEL_VGA_DEVICE(0x2a42, &intel_gm45_info), /* GM45_G */ - INTEL_VGA_DEVICE(0x2e02, &intel_g45_info), /* IGD_E_G */ - INTEL_VGA_DEVICE(0x2e12, &intel_g45_info), /* Q45_G */ - INTEL_VGA_DEVICE(0x2e22, &intel_g45_info), /* G45_G */ - INTEL_VGA_DEVICE(0x2e32, &intel_g45_info), /* G41_G */ - INTEL_VGA_DEVICE(0x2e42, &intel_g45_info), /* B43_G */ - INTEL_VGA_DEVICE(0x2e92, &intel_g45_info), /* B43_G.1 */ - INTEL_VGA_DEVICE(0xa001, &intel_pineview_info), - INTEL_VGA_DEVICE(0xa011, &intel_pineview_info), - INTEL_VGA_DEVICE(0x0042, &intel_ironlake_d_info), - INTEL_VGA_DEVICE(0x0046, &intel_ironlake_m_info), - INTEL_VGA_DEVICE(0x0102, &intel_sandybridge_d_info), - INTEL_VGA_DEVICE(0x0112, &intel_sandybridge_d_info), - INTEL_VGA_DEVICE(0x0122, &intel_sandybridge_d_info), - INTEL_VGA_DEVICE(0x0106, &intel_sandybridge_m_info), - INTEL_VGA_DEVICE(0x0116, &intel_sandybridge_m_info), - INTEL_VGA_DEVICE(0x0126, &intel_sandybridge_m_info), - INTEL_VGA_DEVICE(0x010A, &intel_sandybridge_d_info), - INTEL_VGA_DEVICE(0x0156, &intel_ivybridge_m_info), /* GT1 mobile */ - INTEL_VGA_DEVICE(0x0166, &intel_ivybridge_m_info), /* GT2 mobile */ - INTEL_VGA_DEVICE(0x0152, &intel_ivybridge_d_info), /* GT1 desktop */ - INTEL_VGA_DEVICE(0x0162, &intel_ivybridge_d_info), /* GT2 desktop */ - INTEL_VGA_DEVICE(0x015a, &intel_ivybridge_d_info), /* GT1 server */ - INTEL_QUANTA_VGA_DEVICE(&intel_ivybridge_q_info), /* Quanta transcode */ - INTEL_VGA_DEVICE(0x016a, &intel_ivybridge_d_info), /* GT2 server */ - INTEL_VGA_DEVICE(0x0402, &intel_haswell_d_info), /* GT1 desktop */ - INTEL_VGA_DEVICE(0x0412, &intel_haswell_d_info), /* GT2 desktop */ - INTEL_VGA_DEVICE(0x0422, &intel_haswell_d_info), /* GT3 desktop */ - INTEL_VGA_DEVICE(0x040a, &intel_haswell_d_info), /* GT1 server */ - INTEL_VGA_DEVICE(0x041a, &intel_haswell_d_info), /* GT2 server */ - INTEL_VGA_DEVICE(0x042a, &intel_haswell_d_info), /* GT3 server */ - INTEL_VGA_DEVICE(0x0406, &intel_haswell_m_info), /* GT1 mobile */ - INTEL_VGA_DEVICE(0x0416, &intel_haswell_m_info), /* GT2 mobile */ - INTEL_VGA_DEVICE(0x0426, &intel_haswell_m_info), /* GT2 mobile */ - INTEL_VGA_DEVICE(0x040B, &intel_haswell_d_info), /* GT1 reserved */ - INTEL_VGA_DEVICE(0x041B, &intel_haswell_d_info), /* GT2 reserved */ - INTEL_VGA_DEVICE(0x042B, &intel_haswell_d_info), /* GT3 reserved */ - INTEL_VGA_DEVICE(0x040E, &intel_haswell_d_info), /* GT1 reserved */ - INTEL_VGA_DEVICE(0x041E, &intel_haswell_d_info), /* GT2 reserved */ - INTEL_VGA_DEVICE(0x042E, &intel_haswell_d_info), /* GT3 reserved */ - INTEL_VGA_DEVICE(0x0C02, &intel_haswell_d_info), /* SDV GT1 desktop */ - INTEL_VGA_DEVICE(0x0C12, &intel_haswell_d_info), /* SDV GT2 desktop */ - INTEL_VGA_DEVICE(0x0C22, &intel_haswell_d_info), /* SDV GT3 desktop */ - INTEL_VGA_DEVICE(0x0C0A, &intel_haswell_d_info), /* SDV GT1 server */ - INTEL_VGA_DEVICE(0x0C1A, &intel_haswell_d_info), /* SDV GT2 server */ - INTEL_VGA_DEVICE(0x0C2A, &intel_haswell_d_info), /* SDV GT3 server */ - INTEL_VGA_DEVICE(0x0C06, &intel_haswell_m_info), /* SDV GT1 mobile */ - INTEL_VGA_DEVICE(0x0C16, &intel_haswell_m_info), /* SDV GT2 mobile */ - INTEL_VGA_DEVICE(0x0C26, &intel_haswell_m_info), /* SDV GT3 mobile */ - INTEL_VGA_DEVICE(0x0C0B, &intel_haswell_d_info), /* SDV GT1 reserved */ - INTEL_VGA_DEVICE(0x0C1B, &intel_haswell_d_info), /* SDV GT2 reserved */ - INTEL_VGA_DEVICE(0x0C2B, &intel_haswell_d_info), /* SDV GT3 reserved */ - INTEL_VGA_DEVICE(0x0C0E, &intel_haswell_d_info), /* SDV GT1 reserved */ - INTEL_VGA_DEVICE(0x0C1E, &intel_haswell_d_info), /* SDV GT2 reserved */ - INTEL_VGA_DEVICE(0x0C2E, &intel_haswell_d_info), /* SDV GT3 reserved */ - INTEL_VGA_DEVICE(0x0A02, &intel_haswell_d_info), /* ULT GT1 desktop */ - INTEL_VGA_DEVICE(0x0A12, &intel_haswell_d_info), /* ULT GT2 desktop */ - INTEL_VGA_DEVICE(0x0A22, &intel_haswell_d_info), /* ULT GT3 desktop */ - INTEL_VGA_DEVICE(0x0A0A, &intel_haswell_d_info), /* ULT GT1 server */ - INTEL_VGA_DEVICE(0x0A1A, &intel_haswell_d_info), /* ULT GT2 server */ - INTEL_VGA_DEVICE(0x0A2A, &intel_haswell_d_info), /* ULT GT3 server */ - INTEL_VGA_DEVICE(0x0A06, &intel_haswell_m_info), /* ULT GT1 mobile */ - INTEL_VGA_DEVICE(0x0A16, &intel_haswell_m_info), /* ULT GT2 mobile */ - INTEL_VGA_DEVICE(0x0A26, &intel_haswell_m_info), /* ULT GT3 mobile */ - INTEL_VGA_DEVICE(0x0A0B, &intel_haswell_d_info), /* ULT GT1 reserved */ - INTEL_VGA_DEVICE(0x0A1B, &intel_haswell_d_info), /* ULT GT2 reserved */ - INTEL_VGA_DEVICE(0x0A2B, &intel_haswell_d_info), /* ULT GT3 reserved */ - INTEL_VGA_DEVICE(0x0A0E, &intel_haswell_m_info), /* ULT GT1 reserved */ - INTEL_VGA_DEVICE(0x0A1E, &intel_haswell_m_info), /* ULT GT2 reserved */ - INTEL_VGA_DEVICE(0x0A2E, &intel_haswell_m_info), /* ULT GT3 reserved */ - INTEL_VGA_DEVICE(0x0D02, &intel_haswell_d_info), /* CRW GT1 desktop */ - INTEL_VGA_DEVICE(0x0D12, &intel_haswell_d_info), /* CRW GT2 desktop */ - INTEL_VGA_DEVICE(0x0D22, &intel_haswell_d_info), /* CRW GT3 desktop */ - INTEL_VGA_DEVICE(0x0D0A, &intel_haswell_d_info), /* CRW GT1 server */ - INTEL_VGA_DEVICE(0x0D1A, &intel_haswell_d_info), /* CRW GT2 server */ - INTEL_VGA_DEVICE(0x0D2A, &intel_haswell_d_info), /* CRW GT3 server */ - INTEL_VGA_DEVICE(0x0D06, &intel_haswell_m_info), /* CRW GT1 mobile */ - INTEL_VGA_DEVICE(0x0D16, &intel_haswell_m_info), /* CRW GT2 mobile */ - INTEL_VGA_DEVICE(0x0D26, &intel_haswell_m_info), /* CRW GT3 mobile */ - INTEL_VGA_DEVICE(0x0D0B, &intel_haswell_d_info), /* CRW GT1 reserved */ - INTEL_VGA_DEVICE(0x0D1B, &intel_haswell_d_info), /* CRW GT2 reserved */ - INTEL_VGA_DEVICE(0x0D2B, &intel_haswell_d_info), /* CRW GT3 reserved */ - INTEL_VGA_DEVICE(0x0D0E, &intel_haswell_d_info), /* CRW GT1 reserved */ - INTEL_VGA_DEVICE(0x0D1E, &intel_haswell_d_info), /* CRW GT2 reserved */ - INTEL_VGA_DEVICE(0x0D2E, &intel_haswell_d_info), /* CRW GT3 reserved */ - INTEL_VGA_DEVICE(0x0f30, &intel_valleyview_m_info), - INTEL_VGA_DEVICE(0x0f31, &intel_valleyview_m_info), - INTEL_VGA_DEVICE(0x0f32, &intel_valleyview_m_info), - INTEL_VGA_DEVICE(0x0f33, &intel_valleyview_m_info), - INTEL_VGA_DEVICE(0x0157, &intel_valleyview_m_info), - INTEL_VGA_DEVICE(0x0155, &intel_valleyview_d_info), + INTEL_PCI_IDS, {0, 0, 0} }; diff --git a/include/drm/i915_drm.h b/include/drm/i915_drm.h index 63d609d..7276a72 100644 --- a/include/drm/i915_drm.h +++ b/include/drm/i915_drm.h @@ -26,6 +26,7 @@ #ifndef _I915_DRM_H_ #define _I915_DRM_H_ +#include <drm/i915_pciids.h> #include <uapi/drm/i915_drm.h> /* For use by IPS driver */ @@ -34,4 +35,5 @@ extern bool i915_gpu_raise(void); extern bool i915_gpu_lower(void); extern bool i915_gpu_busy(void); extern bool i915_gpu_turbo_disable(void); + #endif /* _I915_DRM_H_ */ diff --git a/include/drm/i915_pciids.h b/include/drm/i915_pciids.h new file mode 100644 index 0000000..3f03544 --- /dev/null +++ b/include/drm/i915_pciids.h @@ -0,0 +1,208 @@ +/* + * Copyright 2013 Intel Corporation + * All Rights Reserved. + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the + * "Software"), to deal in the Software without restriction, including + * without limitation the rights to use, copy, modify, merge, publish, + * distribute, sub license, and/or sell copies of the Software, and to + * permit persons to whom the Software is furnished to do so, subject to + * the following conditions: + * + * The above copyright notice and this permission notice (including the + * next paragraph) shall be included in all copies or substantial portions + * of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS + * OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. + * IN NO EVENT SHALL TUNGSTEN GRAPHICS AND/OR ITS SUPPLIERS BE LIABLE FOR + * ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, + * TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE + * SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + * + */ +#ifndef _I915_PCIIDS_H +#define _I915_PCIIDS_H + +#define INTEL_VGA_DEVICE(id, info) { \ + .class = PCI_BASE_CLASS_DISPLAY << 16, \ + .class_mask = 0xff0000, \ + .vendor = 0x8086, \ + .device = id, \ + .subvendor = PCI_ANY_ID, \ + .subdevice = PCI_ANY_ID, \ + .driver_data = (unsigned long) info } + +#define INTEL_QUANTA_VGA_DEVICE(info) { \ + .class = PCI_BASE_CLASS_DISPLAY << 16, \ + .class_mask = 0xff0000, \ + .vendor = 0x8086, \ + .device = 0x16a, \ + .subvendor = 0x152d, \ + .subdevice = 0x8990, \ + .driver_data = (unsigned long) info } + +#define INTEL_I830_IDS(info) \ + INTEL_VGA_DEVICE(0x3577, info) + +#define INTEL_I845G_IDS(info) \ + INTEL_VGA_DEVICE(0x2562, info) + +#define INTEL_I85X_IDS(info) \ + INTEL_VGA_DEVICE(0x3582, info), /* I855_GM */ \ + INTEL_VGA_DEVICE(0x358e, info) + +#define INTEL_I865G_IDS(info) \ + INTEL_VGA_DEVICE(0x2572, info) /* I865_G */ + +#define INTEL_I915G_IDS(info) \ + INTEL_VGA_DEVICE(0x2582, info), /* I915_G */ \ + INTEL_VGA_DEVICE(0x258a, info) /* E7221_G */ + +#define INTEL_I915GM_IDS(info) \ + INTEL_VGA_DEVICE(0x2592, info) /* I915_GM */ + +#define INTEL_I945G_IDS(info) \ + INTEL_VGA_DEVICE(0x2772, info) /* I945_G */ + +#define INTEL_I945GM_IDS(info) \ + INTEL_VGA_DEVICE(0x27a2, info), /* I945_GM */ \ + INTEL_VGA_DEVICE(0x27ae, info) /* I945_GME */ + +#define INTEL_I965G_IDS(info) \ + INTEL_VGA_DEVICE(0x2972, info), /* I946_GZ */ \ + INTEL_VGA_DEVICE(0x2982, info), /* G35_G */ \ + INTEL_VGA_DEVICE(0x2992, info), /* I965_Q */ \ + INTEL_VGA_DEVICE(0x29a2, info) /* I965_G */ + +#define INTEL_G33_IDS(info) \ + INTEL_VGA_DEVICE(0x29b2, info), /* Q35_G */ \ + INTEL_VGA_DEVICE(0x29c2, info), /* G33_G */ \ + INTEL_VGA_DEVICE(0x29d2, info) /* Q33_G */ + +#define INTEL_I965GM_IDS(info) \ + INTEL_VGA_DEVICE(0x2a02, info), /* I965_GM */ \ + INTEL_VGA_DEVICE(0x2a12, info) /* I965_GME */ + +#define INTEL_GM45_IDS(info) \ + INTEL_VGA_DEVICE(0x2a42, info) /* GM45_G */ + +#define INTEL_G45_IDS(info) \ + INTEL_VGA_DEVICE(0x2e02, info), /* IGD_E_G */ \ + INTEL_VGA_DEVICE(0x2e12, info), /* Q45_G */ \ + INTEL_VGA_DEVICE(0x2e22, info), /* G45_G */ \ + INTEL_VGA_DEVICE(0x2e32, info), /* G41_G */ \ + INTEL_VGA_DEVICE(0x2e42, info), /* B43_G */ \ + INTEL_VGA_DEVICE(0x2e92, info) /* B43_G.1 */ + +#define INTEL_PINEVIEW_IDS(info) \ + INTEL_VGA_DEVICE(0xa001, info), \ + INTEL_VGA_DEVICE(0xa011, info) + +#define INTEL_IRONLAKE_D_IDS(info) \ + INTEL_VGA_DEVICE(0x0042, info) + +#define INTEL_IRONLAKE_M_IDS(info) \ + INTEL_VGA_DEVICE(0x0046, info) + +#define INTEL_SNB_D_IDS(info) \ + INTEL_VGA_DEVICE(0x0102, info), \ + INTEL_VGA_DEVICE(0x0112, info), \ + INTEL_VGA_DEVICE(0x0122, info), \ + INTEL_VGA_DEVICE(0x010A, info) + +#define INTEL_SNB_M_IDS(info) \ + INTEL_VGA_DEVICE(0x0106, info), \ + INTEL_VGA_DEVICE(0x0116, info), \ + INTEL_VGA_DEVICE(0x0126, info) + +#define INTEL_IVB_M_IDS(info) \ + INTEL_VGA_DEVICE(0x0156, info), /* GT1 mobile */ \ + INTEL_VGA_DEVICE(0x0166, info) /* GT2 mobile */ + +#define INTEL_IVB_D_IDS(info) \ + INTEL_VGA_DEVICE(0x0152, info), /* GT1 desktop */ \ + INTEL_VGA_DEVICE(0x0162, info), /* GT2 desktop */ \ + INTEL_VGA_DEVICE(0x015a, info), /* GT1 server */ \ + INTEL_VGA_DEVICE(0x016a, info) /* GT2 server */ + +#define INTEL_IVB_Q_IDS(info) \ + INTEL_QUANTA_VGA_DEVICE(info) /* Quanta transcode */ + +#define INTEL_HSW_D_IDS(info) \ + INTEL_VGA_DEVICE(0x0402, info), /* GT1 desktop */ \ + INTEL_VGA_DEVICE(0x0412, info), /* GT2 desktop */ \ + INTEL_VGA_DEVICE(0x0422, info), /* GT3 desktop */ \ + INTEL_VGA_DEVICE(0x040a, info), /* GT1 server */ \ + INTEL_VGA_DEVICE(0x041a, info), /* GT2 server */ \ + INTEL_VGA_DEVICE(0x042a, info), /* GT3 server */ \ + INTEL_VGA_DEVICE(0x040B, info), /* GT1 reserved */ \ + INTEL_VGA_DEVICE(0x041B, info), /* GT2 reserved */ \ + INTEL_VGA_DEVICE(0x042B, info), /* GT3 reserved */ \ + INTEL_VGA_DEVICE(0x040E, info), /* GT1 reserved */ \ + INTEL_VGA_DEVICE(0x041E, info), /* GT2 reserved */ \ + INTEL_VGA_DEVICE(0x042E, info), /* GT3 reserved */ \ + INTEL_VGA_DEVICE(0x0C02, info), /* SDV GT1 desktop */ \ + INTEL_VGA_DEVICE(0x0C12, info), /* SDV GT2 desktop */ \ + INTEL_VGA_DEVICE(0x0C22, info), /* SDV GT3 desktop */ \ + INTEL_VGA_DEVICE(0x0C0A, info), /* SDV GT1 server */ \ + INTEL_VGA_DEVICE(0x0C1A, info), /* SDV GT2 server */ \ + INTEL_VGA_DEVICE(0x0C2A, info), /* SDV GT3 server */ \ + INTEL_VGA_DEVICE(0x0C0B, info), /* SDV GT1 reserved */ \ + INTEL_VGA_DEVICE(0x0C1B, info), /* SDV GT2 reserved */ \ + INTEL_VGA_DEVICE(0x0C2B, info), /* SDV GT3 reserved */ \ + INTEL_VGA_DEVICE(0x0C0E, info), /* SDV GT1 reserved */ \ + INTEL_VGA_DEVICE(0x0C1E, info), /* SDV GT2 reserved */ \ + INTEL_VGA_DEVICE(0x0C2E, info), /* SDV GT3 reserved */ \ + INTEL_VGA_DEVICE(0x0A02, info), /* ULT GT1 desktop */ \ + INTEL_VGA_DEVICE(0x0A12, info), /* ULT GT2 desktop */ \ + INTEL_VGA_DEVICE(0x0A22, info), /* ULT GT3 desktop */ \ + INTEL_VGA_DEVICE(0x0A0A, info), /* ULT GT1 server */ \ + INTEL_VGA_DEVICE(0x0A1A, info), /* ULT GT2 server */ \ + INTEL_VGA_DEVICE(0x0A2A, info), /* ULT GT3 server */ \ + INTEL_VGA_DEVICE(0x0A0B, info), /* ULT GT1 reserved */ \ + INTEL_VGA_DEVICE(0x0A1B, info), /* ULT GT2 reserved */ \ + INTEL_VGA_DEVICE(0x0A2B, info), /* ULT GT3 reserved */ \ + INTEL_VGA_DEVICE(0x0D02, info), /* CRW GT1 desktop */ \ + INTEL_VGA_DEVICE(0x0D12, info), /* CRW GT2 desktop */ \ + INTEL_VGA_DEVICE(0x0D22, info), /* CRW GT3 desktop */ \ + INTEL_VGA_DEVICE(0x0D0A, info), /* CRW GT1 server */ \ + INTEL_VGA_DEVICE(0x0D1A, info), /* CRW GT2 server */ \ + INTEL_VGA_DEVICE(0x0D2A, info), /* CRW GT3 server */ \ + INTEL_VGA_DEVICE(0x0D0B, info), /* CRW GT1 reserved */ \ + INTEL_VGA_DEVICE(0x0D1B, info), /* CRW GT2 reserved */ \ + INTEL_VGA_DEVICE(0x0D2B, info), /* CRW GT3 reserved */ \ + INTEL_VGA_DEVICE(0x0D0E, info), /* CRW GT1 reserved */ \ + INTEL_VGA_DEVICE(0x0D1E, info), /* CRW GT2 reserved */ \ + INTEL_VGA_DEVICE(0x0D2E, info) /* CRW GT3 reserved */ \ + +#define INTEL_HSW_M_IDS(info) \ + INTEL_VGA_DEVICE(0x0406, info), /* GT1 mobile */ \ + INTEL_VGA_DEVICE(0x0416, info), /* GT2 mobile */ \ + INTEL_VGA_DEVICE(0x0426, info), /* GT2 mobile */ \ + INTEL_VGA_DEVICE(0x0C06, info), /* SDV GT1 mobile */ \ + INTEL_VGA_DEVICE(0x0C16, info), /* SDV GT2 mobile */ \ + INTEL_VGA_DEVICE(0x0C26, info), /* SDV GT3 mobile */ \ + INTEL_VGA_DEVICE(0x0A06, info), /* ULT GT1 mobile */ \ + INTEL_VGA_DEVICE(0x0A16, info), /* ULT GT2 mobile */ \ + INTEL_VGA_DEVICE(0x0A26, info), /* ULT GT3 mobile */ \ + INTEL_VGA_DEVICE(0x0A0E, info), /* ULT GT1 reserved */ \ + INTEL_VGA_DEVICE(0x0A1E, info), /* ULT GT2 reserved */ \ + INTEL_VGA_DEVICE(0x0A2E, info), /* ULT GT3 reserved */ \ + INTEL_VGA_DEVICE(0x0D06, info), /* CRW GT1 mobile */ \ + INTEL_VGA_DEVICE(0x0D16, info), /* CRW GT2 mobile */ \ + INTEL_VGA_DEVICE(0x0D26, info) /* CRW GT3 mobile */ + +#define INTEL_VLV_M_IDS(info) \ + INTEL_VGA_DEVICE(0x0f30, info), \ + INTEL_VGA_DEVICE(0x0f31, info), \ + INTEL_VGA_DEVICE(0x0f32, info), \ + INTEL_VGA_DEVICE(0x0f33, info), \ + INTEL_VGA_DEVICE(0x0157, info) + +#define INTEL_VLV_D_IDS(info) \ + INTEL_VGA_DEVICE(0x0155, info) + +#endif /* _I915_PCIIDS_H */ -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 1/2] drm/i915: split PCI IDs out into i915_drm.h v3 2013-07-25 16:37 ` [PATCH 1/2] drm/i915: split PCI IDs out into i915_drm.h v3 Jesse Barnes @ 2013-07-25 23:06 ` Chris Wilson 0 siblings, 0 replies; 20+ messages in thread From: Chris Wilson @ 2013-07-25 23:06 UTC (permalink / raw) To: Jesse Barnes; +Cc: intel-gfx, linux-kernel, hpa, mingo, tglx, torvalds On Thu, Jul 25, 2013 at 09:37:48AM -0700, Jesse Barnes wrote: > For use by userspace (at some point in the future) and other kernel code. > > v2: move PCI IDs to uabi (Chris) > move PCI IDs to drm/ (Dave) > v3: fixup Quanta detection - needs to come first (Daniel) One last comment! > +#define INTEL_VGA_DEVICE(id, info) { \ > + .class = PCI_BASE_CLASS_DISPLAY << 16, \ > + .class_mask = 0xff0000, \ > + .vendor = 0x8086, \ > + .device = id, \ > + .subvendor = PCI_ANY_ID, \ > + .subdevice = PCI_ANY_ID, \ > + .driver_data = (unsigned long) info } libpciaccess doesn't define either PCI_BASE_CLASS_DISPLAY or PCI_ANY_ID. Since we use the hex values for the rest of the elements, it would seem to be tidier to also use 0x3 << 16 instead of PCI_BASE_CLASS_DISPLAY << 16. I would leave PCI_ANY_ID as a symbolic value though. -Chris -- Chris Wilson, Intel Open Source Technology Centre ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 2/2] x86: add early quirk for reserving Intel graphics stolen memory v3 2013-07-25 16:37 Ugly patches for stolen reservation Jesse Barnes 2013-07-25 16:37 ` [PATCH 1/2] drm/i915: split PCI IDs out into i915_drm.h v3 Jesse Barnes @ 2013-07-25 16:37 ` Jesse Barnes 2013-07-25 22:42 ` Chris Wilson ` (2 more replies) 2013-07-25 20:05 ` Ugly patches for stolen reservation Ingo Molnar 2 siblings, 3 replies; 20+ messages in thread From: Jesse Barnes @ 2013-07-25 16:37 UTC (permalink / raw) To: intel-gfx, linux-kernel; +Cc: hpa, mingo, tglx, torvalds Systems with Intel graphics controllers set aside memory exclusively for gfx driver use. This memory is not marked in the E820 as reserved or as RAM, and so is subject to overlap from E820 manipulation later in the boot process. On some systems, MMIO space is allocated on top, despite the efforts of the "RAM buffer" approach, which simply rounds memory boundaries up to 64M to try to catch space that may decode as RAM and so is not suitable for MMIO. v2: use read_pci_config for 32 bit reads instead of adding a new one (Chris) add gen6 stolen size function (Chris) use a function pointer (Chris) drop gen2 bits (Daniel) Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> --- arch/x86/kernel/early-quirks.c | 158 +++++++++++++++++++++++++++++++++++++++ drivers/gpu/drm/i915/i915_reg.h | 15 ---- include/drm/i915_drm.h | 32 ++++++++ 3 files changed, 190 insertions(+), 15 deletions(-) diff --git a/arch/x86/kernel/early-quirks.c b/arch/x86/kernel/early-quirks.c index 94ab6b9..bff8a6f 100644 --- a/arch/x86/kernel/early-quirks.c +++ b/arch/x86/kernel/early-quirks.c @@ -12,6 +12,7 @@ #include <linux/pci.h> #include <linux/acpi.h> #include <linux/pci_ids.h> +#include <drm/i915_drm.h> #include <asm/pci-direct.h> #include <asm/dma.h> #include <asm/io_apic.h> @@ -208,6 +209,161 @@ static void __init intel_remapping_check(int num, int slot, int func) } +/* + * Systems with Intel graphics controllers set aside memory exclusively + * for gfx driver use. This memory is not marked in the E820 as reserved + * or as RAM, and so is subject to overlap from E820 manipulation later + * in the boot process. On some systems, MMIO space is allocated on top, + * despite the efforts of the "RAM buffer" approach, which simply rounds + * memory boundaries up to 64M to try to catch space that may decode + * as RAM and so is not suitable for MMIO. + * + * And yes, so far on current devices the base addr is always under 4G. + */ +static u32 __init intel_stolen_base(int num, int slot, int func) +{ + u32 base; + + /* + * Almost universally we can find the Graphics Base of Stolen Memory + * at offset 0x5c in the igfx configuration space. On a few (desktop) + * machines this is also mirrored in the bridge device at different + * locations, or in the MCHBAR. + */ + base = read_pci_config(num, slot, func, 0x5c); + base &= ~((1<<20) - 1); + + return base; +} + +#define KB(x) ((x) * 1024) +#define MB(x) (KB (KB (x))) +#define GB(x) (MB (KB (x))) + +static size_t __init gen3_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 & I855_GMCH_GMS_MASK) { + case I855_GMCH_GMS_STOLEN_1M: + stolen_size = MB(1); + break; + case I855_GMCH_GMS_STOLEN_4M: + stolen_size = MB(4); + break; + case I855_GMCH_GMS_STOLEN_8M: + stolen_size = MB(8); + break; + case I855_GMCH_GMS_STOLEN_16M: + stolen_size = MB(16); + break; + case I855_GMCH_GMS_STOLEN_32M: + stolen_size = MB(32); + break; + case I915_GMCH_GMS_STOLEN_48M: + stolen_size = MB(48); + break; + case I915_GMCH_GMS_STOLEN_64M: + stolen_size = MB(64); + break; + case G33_GMCH_GMS_STOLEN_128M: + stolen_size = MB(128); + break; + case G33_GMCH_GMS_STOLEN_256M: + stolen_size = MB(256); + break; + case INTEL_GMCH_GMS_STOLEN_96M: + stolen_size = MB(96); + break; + case INTEL_GMCH_GMS_STOLEN_160M: + stolen_size = MB(160); + break; + case INTEL_GMCH_GMS_STOLEN_224M: + stolen_size = MB(224); + break; + case INTEL_GMCH_GMS_STOLEN_352M: + stolen_size = MB(352); + break; + default: + stolen_size = 0; + break; + } + + return stolen_size; +} + +static size_t __init gen6_stolen_size(int num, int slot, int func) +{ + u16 gmch_ctrl; + + gmch_ctrl = read_pci_config_16(num, slot, func, SNB_GMCH_CTRL); + gmch_ctrl >>= SNB_GMCH_GMS_SHIFT; + gmch_ctrl &= SNB_GMCH_GMS_MASK; + + return gmch_ctrl << 25; /* 32 MB units */ +} + +typedef size_t (*stolen_size_fn)(int num, int slot, int func); + +static struct pci_device_id intel_stolen_ids[] __initdata = { + INTEL_I915G_IDS(gen3_stolen_size), + INTEL_I915GM_IDS(gen3_stolen_size), + INTEL_I945G_IDS(gen3_stolen_size), + INTEL_I945GM_IDS(gen3_stolen_size), + INTEL_VLV_M_IDS(gen3_stolen_size), + INTEL_VLV_D_IDS(gen3_stolen_size), + INTEL_PINEVIEW_IDS(gen3_stolen_size), + INTEL_I965G_IDS(gen3_stolen_size), + INTEL_G33_IDS(gen3_stolen_size), + INTEL_I965GM_IDS(gen3_stolen_size), + INTEL_GM45_IDS(gen3_stolen_size), + INTEL_G45_IDS(gen3_stolen_size), + INTEL_IRONLAKE_D_IDS(gen3_stolen_size), + INTEL_IRONLAKE_M_IDS(gen3_stolen_size), + INTEL_SNB_D_IDS(gen6_stolen_size), + INTEL_SNB_M_IDS(gen6_stolen_size), + INTEL_IVB_M_IDS(gen6_stolen_size), + INTEL_IVB_D_IDS(gen6_stolen_size), + INTEL_HSW_D_IDS(gen6_stolen_size), + INTEL_HSW_M_IDS(gen6_stolen_size), +}; + +static void __init intel_graphics_stolen(int num, int slot, int func) +{ + size_t size; + int i; + u32 start; + u16 device, subvendor, subdevice; + + device = read_pci_config_16(num, slot, func, PCI_DEVICE_ID); + subvendor = read_pci_config_16(num, slot, func, + PCI_SUBSYSTEM_VENDOR_ID); + subdevice = read_pci_config_16(num, slot, func, PCI_SUBSYSTEM_ID); + + for (i = 0; i < ARRAY_SIZE(intel_stolen_ids); i++) { + if (intel_stolen_ids[i].device == device) { + stolen_size_fn stolen_size = + (stolen_size_fn)intel_stolen_ids[i].driver_data; + size = stolen_size(num, slot, func); + start = intel_stolen_base(num, slot, func); + if (size && start) + goto found; + else + break; + } + } + + /* No match or invalid data, don't bother reserving */ + return; +found: + /* Mark this space as reserved */ + e820_add_region(start, size, E820_RESERVED); + return; +} + #define QFLAG_APPLY_ONCE 0x1 #define QFLAG_APPLIED 0x2 #define QFLAG_DONE (QFLAG_APPLY_ONCE|QFLAG_APPLIED) @@ -241,6 +397,8 @@ static struct chipset early_qrk[] __initdata = { PCI_BASE_CLASS_BRIDGE, 0, intel_remapping_check }, { PCI_VENDOR_ID_INTEL, 0x3406, PCI_CLASS_BRIDGE_HOST, PCI_BASE_CLASS_BRIDGE, 0, intel_remapping_check }, + { PCI_VENDOR_ID_INTEL, PCI_ANY_ID, PCI_CLASS_DISPLAY_VGA, PCI_ANY_ID, + QFLAG_APPLY_ONCE, intel_graphics_stolen }, {} }; diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 5e58a44..d707ce6 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -33,21 +33,6 @@ #define _MASKED_BIT_ENABLE(a) (((a) << 16) | (a)) #define _MASKED_BIT_DISABLE(a) ((a) << 16) -/* - * The Bridge device's PCI config space has information about the - * fb aperture size and the amount of pre-reserved memory. - * This is all handled in the intel-gtt.ko module. i915.ko only - * cares about the vga bit for the vga rbiter. - */ -#define INTEL_GMCH_CTRL 0x52 -#define INTEL_GMCH_VGA_DISABLE (1 << 1) -#define SNB_GMCH_CTRL 0x50 -#define SNB_GMCH_GGMS_SHIFT 8 /* GTT Graphics Memory Size */ -#define SNB_GMCH_GGMS_MASK 0x3 -#define SNB_GMCH_GMS_SHIFT 3 /* Graphics Mode Select */ -#define SNB_GMCH_GMS_MASK 0x1f - - /* PCI config space */ #define HPLLCC 0xc0 /* 855 only */ diff --git a/include/drm/i915_drm.h b/include/drm/i915_drm.h index 7276a72..3abfa6e 100644 --- a/include/drm/i915_drm.h +++ b/include/drm/i915_drm.h @@ -36,4 +36,36 @@ extern bool i915_gpu_lower(void); extern bool i915_gpu_busy(void); extern bool i915_gpu_turbo_disable(void); +/* + * The Bridge device's PCI config space has information about the + * fb aperture size and the amount of pre-reserved memory. + * This is all handled in the intel-gtt.ko module. i915.ko only + * cares about the vga bit for the vga rbiter. + */ +#define INTEL_GMCH_CTRL 0x52 +#define INTEL_GMCH_VGA_DISABLE (1 << 1) +#define SNB_GMCH_CTRL 0x50 +#define SNB_GMCH_GGMS_SHIFT 8 /* GTT Graphics Memory Size */ +#define SNB_GMCH_GGMS_MASK 0x3 +#define SNB_GMCH_GMS_SHIFT 3 /* Graphics Mode Select */ +#define SNB_GMCH_GMS_MASK 0x1f + +#define I830_GMCH_CTRL 0x52 + +#define I855_GMCH_GMS_MASK 0xF0 +#define I855_GMCH_GMS_STOLEN_0M 0x0 +#define I855_GMCH_GMS_STOLEN_1M (0x1 << 4) +#define I855_GMCH_GMS_STOLEN_4M (0x2 << 4) +#define I855_GMCH_GMS_STOLEN_8M (0x3 << 4) +#define I855_GMCH_GMS_STOLEN_16M (0x4 << 4) +#define I855_GMCH_GMS_STOLEN_32M (0x5 << 4) +#define I915_GMCH_GMS_STOLEN_48M (0x6 << 4) +#define I915_GMCH_GMS_STOLEN_64M (0x7 << 4) +#define G33_GMCH_GMS_STOLEN_128M (0x8 << 4) +#define G33_GMCH_GMS_STOLEN_256M (0x9 << 4) +#define INTEL_GMCH_GMS_STOLEN_96M (0xa << 4) +#define INTEL_GMCH_GMS_STOLEN_160M (0xb << 4) +#define INTEL_GMCH_GMS_STOLEN_224M (0xc << 4) +#define INTEL_GMCH_GMS_STOLEN_352M (0xd << 4) + #endif /* _I915_DRM_H_ */ -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] x86: add early quirk for reserving Intel graphics stolen memory v3 2013-07-25 16:37 ` [PATCH 2/2] x86: add early quirk for reserving Intel graphics stolen memory v3 Jesse Barnes @ 2013-07-25 22:42 ` Chris Wilson 2013-07-25 22:59 ` Chris Wilson 2013-07-26 8:10 ` [Intel-gfx] " Daniel Vetter 2013-07-26 16:58 ` H. Peter Anvin 2 siblings, 1 reply; 20+ messages in thread From: Chris Wilson @ 2013-07-25 22:42 UTC (permalink / raw) To: Jesse Barnes; +Cc: intel-gfx, linux-kernel, hpa, mingo, tglx, torvalds On Thu, Jul 25, 2013 at 09:37:49AM -0700, Jesse Barnes wrote: > Systems with Intel graphics controllers set aside memory exclusively for > gfx driver use. This memory is not marked in the E820 as reserved or as > RAM, and so is subject to overlap from E820 manipulation later in the > boot process. On some systems, MMIO space is allocated on top, despite > the efforts of the "RAM buffer" approach, which simply rounds memory > boundaries up to 64M to try to catch space that may decode as RAM and so > is not suitable for MMIO. > > v2: use read_pci_config for 32 bit reads instead of adding a new one > (Chris) > add gen6 stolen size function (Chris) > use a function pointer (Chris) > drop gen2 bits (Daniel) As a reminder, can you please reorder the entries in the macro to ease compilation by userspace - c++ doesn't like having .class and so I have to massage the file with intel_pciids.h: i915_pciids.h sed 's/_I915_PCIIDS/INTEL_PCIIDS/; s/\..*= //' < $^ > $@ to strip out the C99-style struct declarations (as my cpp skills were obviously not strong enough) but then that requires the member ordering is correct. Alternatively, if we discard the clean C99 code, both the kernel struct pci_device_id and libpciaccess's struct pci_match_data are the same - and I can just use i915_pciids.h directly. My perference is to keep using C99 in the kernel as it makes the header much more readable. -Chris -- Chris Wilson, Intel Open Source Technology Centre ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] x86: add early quirk for reserving Intel graphics stolen memory v3 2013-07-25 22:42 ` Chris Wilson @ 2013-07-25 22:59 ` Chris Wilson 2013-07-25 23:18 ` Jesse Barnes 0 siblings, 1 reply; 20+ messages in thread From: Chris Wilson @ 2013-07-25 22:59 UTC (permalink / raw) To: Jesse Barnes, intel-gfx, linux-kernel, hpa, mingo, tglx, torvalds Hmm, interesting licence block in i915_pciids.h - our claim to grant licence but TG disclaims liability. -Chris -- Chris Wilson, Intel Open Source Technology Centre ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] x86: add early quirk for reserving Intel graphics stolen memory v3 2013-07-25 22:59 ` Chris Wilson @ 2013-07-25 23:18 ` Jesse Barnes 0 siblings, 0 replies; 20+ messages in thread From: Jesse Barnes @ 2013-07-25 23:18 UTC (permalink / raw) To: Chris Wilson; +Cc: intel-gfx, linux-kernel, hpa, mingo, tglx, torvalds On Thu, 25 Jul 2013 23:59:25 +0100 Chris Wilson <chris@chris-wilson.co.uk> wrote: > Hmm, interesting licence block in i915_pciids.h - our claim to > grant licence but TG disclaims liability. Oops my manual search & replace obviously failed. Will fix up. -- Jesse Barnes, Intel Open Source Technology Center ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Intel-gfx] [PATCH 2/2] x86: add early quirk for reserving Intel graphics stolen memory v3 2013-07-25 16:37 ` [PATCH 2/2] x86: add early quirk for reserving Intel graphics stolen memory v3 Jesse Barnes 2013-07-25 22:42 ` Chris Wilson @ 2013-07-26 8:10 ` Daniel Vetter 2013-07-26 16:58 ` H. Peter Anvin 2 siblings, 0 replies; 20+ messages in thread From: Daniel Vetter @ 2013-07-26 8:10 UTC (permalink / raw) To: Jesse Barnes; +Cc: intel-gfx, linux-kernel, mingo, torvalds, tglx, hpa On Thu, Jul 25, 2013 at 09:37:49AM -0700, Jesse Barnes wrote: > Systems with Intel graphics controllers set aside memory exclusively for > gfx driver use. This memory is not marked in the E820 as reserved or as > RAM, and so is subject to overlap from E820 manipulation later in the > boot process. On some systems, MMIO space is allocated on top, despite > the efforts of the "RAM buffer" approach, which simply rounds memory > boundaries up to 64M to try to catch space that may decode as RAM and so > is not suitable for MMIO. > > v2: use read_pci_config for 32 bit reads instead of adding a new one > (Chris) > add gen6 stolen size function (Chris) > use a function pointer (Chris) > drop gen2 bits (Daniel) > > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> Our QA seems to be happy with this. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=66844 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=66726 Tested-by: lu hua <huax.lu@intel.com> Cheers, Daniel > --- > arch/x86/kernel/early-quirks.c | 158 +++++++++++++++++++++++++++++++++++++++ > drivers/gpu/drm/i915/i915_reg.h | 15 ---- > include/drm/i915_drm.h | 32 ++++++++ > 3 files changed, 190 insertions(+), 15 deletions(-) > > diff --git a/arch/x86/kernel/early-quirks.c b/arch/x86/kernel/early-quirks.c > index 94ab6b9..bff8a6f 100644 > --- a/arch/x86/kernel/early-quirks.c > +++ b/arch/x86/kernel/early-quirks.c > @@ -12,6 +12,7 @@ > #include <linux/pci.h> > #include <linux/acpi.h> > #include <linux/pci_ids.h> > +#include <drm/i915_drm.h> > #include <asm/pci-direct.h> > #include <asm/dma.h> > #include <asm/io_apic.h> > @@ -208,6 +209,161 @@ static void __init intel_remapping_check(int num, int slot, int func) > > } > > +/* > + * Systems with Intel graphics controllers set aside memory exclusively > + * for gfx driver use. This memory is not marked in the E820 as reserved > + * or as RAM, and so is subject to overlap from E820 manipulation later > + * in the boot process. On some systems, MMIO space is allocated on top, > + * despite the efforts of the "RAM buffer" approach, which simply rounds > + * memory boundaries up to 64M to try to catch space that may decode > + * as RAM and so is not suitable for MMIO. > + * > + * And yes, so far on current devices the base addr is always under 4G. > + */ > +static u32 __init intel_stolen_base(int num, int slot, int func) > +{ > + u32 base; > + > + /* > + * Almost universally we can find the Graphics Base of Stolen Memory > + * at offset 0x5c in the igfx configuration space. On a few (desktop) > + * machines this is also mirrored in the bridge device at different > + * locations, or in the MCHBAR. > + */ > + base = read_pci_config(num, slot, func, 0x5c); > + base &= ~((1<<20) - 1); > + > + return base; > +} > + > +#define KB(x) ((x) * 1024) > +#define MB(x) (KB (KB (x))) > +#define GB(x) (MB (KB (x))) > + > +static size_t __init gen3_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 & I855_GMCH_GMS_MASK) { > + case I855_GMCH_GMS_STOLEN_1M: > + stolen_size = MB(1); > + break; > + case I855_GMCH_GMS_STOLEN_4M: > + stolen_size = MB(4); > + break; > + case I855_GMCH_GMS_STOLEN_8M: > + stolen_size = MB(8); > + break; > + case I855_GMCH_GMS_STOLEN_16M: > + stolen_size = MB(16); > + break; > + case I855_GMCH_GMS_STOLEN_32M: > + stolen_size = MB(32); > + break; > + case I915_GMCH_GMS_STOLEN_48M: > + stolen_size = MB(48); > + break; > + case I915_GMCH_GMS_STOLEN_64M: > + stolen_size = MB(64); > + break; > + case G33_GMCH_GMS_STOLEN_128M: > + stolen_size = MB(128); > + break; > + case G33_GMCH_GMS_STOLEN_256M: > + stolen_size = MB(256); > + break; > + case INTEL_GMCH_GMS_STOLEN_96M: > + stolen_size = MB(96); > + break; > + case INTEL_GMCH_GMS_STOLEN_160M: > + stolen_size = MB(160); > + break; > + case INTEL_GMCH_GMS_STOLEN_224M: > + stolen_size = MB(224); > + break; > + case INTEL_GMCH_GMS_STOLEN_352M: > + stolen_size = MB(352); > + break; > + default: > + stolen_size = 0; > + break; > + } > + > + return stolen_size; > +} > + > +static size_t __init gen6_stolen_size(int num, int slot, int func) > +{ > + u16 gmch_ctrl; > + > + gmch_ctrl = read_pci_config_16(num, slot, func, SNB_GMCH_CTRL); > + gmch_ctrl >>= SNB_GMCH_GMS_SHIFT; > + gmch_ctrl &= SNB_GMCH_GMS_MASK; > + > + return gmch_ctrl << 25; /* 32 MB units */ > +} > + > +typedef size_t (*stolen_size_fn)(int num, int slot, int func); > + > +static struct pci_device_id intel_stolen_ids[] __initdata = { > + INTEL_I915G_IDS(gen3_stolen_size), > + INTEL_I915GM_IDS(gen3_stolen_size), > + INTEL_I945G_IDS(gen3_stolen_size), > + INTEL_I945GM_IDS(gen3_stolen_size), > + INTEL_VLV_M_IDS(gen3_stolen_size), > + INTEL_VLV_D_IDS(gen3_stolen_size), > + INTEL_PINEVIEW_IDS(gen3_stolen_size), > + INTEL_I965G_IDS(gen3_stolen_size), > + INTEL_G33_IDS(gen3_stolen_size), > + INTEL_I965GM_IDS(gen3_stolen_size), > + INTEL_GM45_IDS(gen3_stolen_size), > + INTEL_G45_IDS(gen3_stolen_size), > + INTEL_IRONLAKE_D_IDS(gen3_stolen_size), > + INTEL_IRONLAKE_M_IDS(gen3_stolen_size), > + INTEL_SNB_D_IDS(gen6_stolen_size), > + INTEL_SNB_M_IDS(gen6_stolen_size), > + INTEL_IVB_M_IDS(gen6_stolen_size), > + INTEL_IVB_D_IDS(gen6_stolen_size), > + INTEL_HSW_D_IDS(gen6_stolen_size), > + INTEL_HSW_M_IDS(gen6_stolen_size), > +}; > + > +static void __init intel_graphics_stolen(int num, int slot, int func) > +{ > + size_t size; > + int i; > + u32 start; > + u16 device, subvendor, subdevice; > + > + device = read_pci_config_16(num, slot, func, PCI_DEVICE_ID); > + subvendor = read_pci_config_16(num, slot, func, > + PCI_SUBSYSTEM_VENDOR_ID); > + subdevice = read_pci_config_16(num, slot, func, PCI_SUBSYSTEM_ID); > + > + for (i = 0; i < ARRAY_SIZE(intel_stolen_ids); i++) { > + if (intel_stolen_ids[i].device == device) { > + stolen_size_fn stolen_size = > + (stolen_size_fn)intel_stolen_ids[i].driver_data; > + size = stolen_size(num, slot, func); > + start = intel_stolen_base(num, slot, func); > + if (size && start) > + goto found; > + else > + break; > + } > + } > + > + /* No match or invalid data, don't bother reserving */ > + return; > +found: > + /* Mark this space as reserved */ > + e820_add_region(start, size, E820_RESERVED); > + return; > +} > + > #define QFLAG_APPLY_ONCE 0x1 > #define QFLAG_APPLIED 0x2 > #define QFLAG_DONE (QFLAG_APPLY_ONCE|QFLAG_APPLIED) > @@ -241,6 +397,8 @@ static struct chipset early_qrk[] __initdata = { > PCI_BASE_CLASS_BRIDGE, 0, intel_remapping_check }, > { PCI_VENDOR_ID_INTEL, 0x3406, PCI_CLASS_BRIDGE_HOST, > PCI_BASE_CLASS_BRIDGE, 0, intel_remapping_check }, > + { PCI_VENDOR_ID_INTEL, PCI_ANY_ID, PCI_CLASS_DISPLAY_VGA, PCI_ANY_ID, > + QFLAG_APPLY_ONCE, intel_graphics_stolen }, > {} > }; > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index 5e58a44..d707ce6 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -33,21 +33,6 @@ > #define _MASKED_BIT_ENABLE(a) (((a) << 16) | (a)) > #define _MASKED_BIT_DISABLE(a) ((a) << 16) > > -/* > - * The Bridge device's PCI config space has information about the > - * fb aperture size and the amount of pre-reserved memory. > - * This is all handled in the intel-gtt.ko module. i915.ko only > - * cares about the vga bit for the vga rbiter. > - */ > -#define INTEL_GMCH_CTRL 0x52 > -#define INTEL_GMCH_VGA_DISABLE (1 << 1) > -#define SNB_GMCH_CTRL 0x50 > -#define SNB_GMCH_GGMS_SHIFT 8 /* GTT Graphics Memory Size */ > -#define SNB_GMCH_GGMS_MASK 0x3 > -#define SNB_GMCH_GMS_SHIFT 3 /* Graphics Mode Select */ > -#define SNB_GMCH_GMS_MASK 0x1f > - > - > /* PCI config space */ > > #define HPLLCC 0xc0 /* 855 only */ > diff --git a/include/drm/i915_drm.h b/include/drm/i915_drm.h > index 7276a72..3abfa6e 100644 > --- a/include/drm/i915_drm.h > +++ b/include/drm/i915_drm.h > @@ -36,4 +36,36 @@ extern bool i915_gpu_lower(void); > extern bool i915_gpu_busy(void); > extern bool i915_gpu_turbo_disable(void); > > +/* > + * The Bridge device's PCI config space has information about the > + * fb aperture size and the amount of pre-reserved memory. > + * This is all handled in the intel-gtt.ko module. i915.ko only > + * cares about the vga bit for the vga rbiter. > + */ > +#define INTEL_GMCH_CTRL 0x52 > +#define INTEL_GMCH_VGA_DISABLE (1 << 1) > +#define SNB_GMCH_CTRL 0x50 > +#define SNB_GMCH_GGMS_SHIFT 8 /* GTT Graphics Memory Size */ > +#define SNB_GMCH_GGMS_MASK 0x3 > +#define SNB_GMCH_GMS_SHIFT 3 /* Graphics Mode Select */ > +#define SNB_GMCH_GMS_MASK 0x1f > + > +#define I830_GMCH_CTRL 0x52 > + > +#define I855_GMCH_GMS_MASK 0xF0 > +#define I855_GMCH_GMS_STOLEN_0M 0x0 > +#define I855_GMCH_GMS_STOLEN_1M (0x1 << 4) > +#define I855_GMCH_GMS_STOLEN_4M (0x2 << 4) > +#define I855_GMCH_GMS_STOLEN_8M (0x3 << 4) > +#define I855_GMCH_GMS_STOLEN_16M (0x4 << 4) > +#define I855_GMCH_GMS_STOLEN_32M (0x5 << 4) > +#define I915_GMCH_GMS_STOLEN_48M (0x6 << 4) > +#define I915_GMCH_GMS_STOLEN_64M (0x7 << 4) > +#define G33_GMCH_GMS_STOLEN_128M (0x8 << 4) > +#define G33_GMCH_GMS_STOLEN_256M (0x9 << 4) > +#define INTEL_GMCH_GMS_STOLEN_96M (0xa << 4) > +#define INTEL_GMCH_GMS_STOLEN_160M (0xb << 4) > +#define INTEL_GMCH_GMS_STOLEN_224M (0xc << 4) > +#define INTEL_GMCH_GMS_STOLEN_352M (0xd << 4) > + > #endif /* _I915_DRM_H_ */ > -- > 1.7.9.5 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] x86: add early quirk for reserving Intel graphics stolen memory v3 2013-07-25 16:37 ` [PATCH 2/2] x86: add early quirk for reserving Intel graphics stolen memory v3 Jesse Barnes 2013-07-25 22:42 ` Chris Wilson 2013-07-26 8:10 ` [Intel-gfx] " Daniel Vetter @ 2013-07-26 16:58 ` H. Peter Anvin 2013-07-26 17:12 ` Jesse Barnes 2 siblings, 1 reply; 20+ messages in thread From: H. Peter Anvin @ 2013-07-26 16:58 UTC (permalink / raw) To: Jesse Barnes; +Cc: intel-gfx, linux-kernel, mingo, tglx, torvalds On 07/25/2013 09:37 AM, Jesse Barnes wrote: > Systems with Intel graphics controllers set aside memory exclusively for > + /* > + * Almost universally we can find the Graphics Base of Stolen Memory > + * at offset 0x5c in the igfx configuration space. On a few (desktop) > + * machines this is also mirrored in the bridge device at different > + * locations, or in the MCHBAR. > + */ This comment makes me nervous. It isn't clear to me if it is saying: - All igfx devices has the graphics base at 0x5c, a few have it in other places, too (which doesn't matter, we can use 0x5c anyway), or - Most igfx devices have the graphics base at 0x5c, some don't, and we hope and pray we're not on one of those systems because we're not checking. I assume it is the former, but it really needs to be phrased better. -hpa ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] x86: add early quirk for reserving Intel graphics stolen memory v3 2013-07-26 16:58 ` H. Peter Anvin @ 2013-07-26 17:12 ` Jesse Barnes 0 siblings, 0 replies; 20+ messages in thread From: Jesse Barnes @ 2013-07-26 17:12 UTC (permalink / raw) To: H. Peter Anvin; +Cc: intel-gfx, linux-kernel, mingo, tglx, torvalds On Fri, 26 Jul 2013 09:58:45 -0700 "H. Peter Anvin" <hpa@zytor.com> wrote: > On 07/25/2013 09:37 AM, Jesse Barnes wrote: > > Systems with Intel graphics controllers set aside memory exclusively for > > + /* > > + * Almost universally we can find the Graphics Base of Stolen Memory > > + * at offset 0x5c in the igfx configuration space. On a few (desktop) > > + * machines this is also mirrored in the bridge device at different > > + * locations, or in the MCHBAR. > > + */ > > This comment makes me nervous. It isn't clear to me if it is saying: > > - All igfx devices has the graphics base at 0x5c, a few have it in > other places, too (which doesn't matter, we can use 0x5c anyway), or > - Most igfx devices have the graphics base at 0x5c, some don't, and we > hope and pray we're not on one of those systems because we're not > checking. > > I assume it is the former, but it really needs to be phrased better. I mostly copied it from the driver where we had some other ways of finding it too. For the PCI IDs listed, we can always use 0x5c. I'll fix the comment. -- Jesse Barnes, Intel Open Source Technology Center ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: Ugly patches for stolen reservation 2013-07-25 16:37 Ugly patches for stolen reservation Jesse Barnes 2013-07-25 16:37 ` [PATCH 1/2] drm/i915: split PCI IDs out into i915_drm.h v3 Jesse Barnes 2013-07-25 16:37 ` [PATCH 2/2] x86: add early quirk for reserving Intel graphics stolen memory v3 Jesse Barnes @ 2013-07-25 20:05 ` Ingo Molnar 2013-07-25 20:16 ` Jesse Barnes 2013-07-25 22:42 ` H. Peter Anvin 2 siblings, 2 replies; 20+ messages in thread From: Ingo Molnar @ 2013-07-25 20:05 UTC (permalink / raw) To: Jesse Barnes; +Cc: intel-gfx, linux-kernel, hpa, mingo, tglx, torvalds * Jesse Barnes <jbarnes@virtuousgeek.org> wrote: > Patch 2/2 has the description, but suffice it to say I'm > not really pleased with this, though it does solve a > problem we have. On some machines, we get MMIO space > allocated on top of this hidden memory, which can cause > problems. I'm not sure if there are similar problems for > other hunks of the address space; if so it's possible > this could be made more general (though the bits for > looking up the address of this region are definitely > Intel graphics specific). It looks pretty hardware specific. Discovering it the hard way and marking it e820 reserved in an early quirk is what the firmware should have done to begin with - and I doubt the kernel could do anything significantly cleaner. How does Windows manage to not crash? By luckily never allocating PCI resources on top of the RAM? Or does it have a quirk? > Chris has some patches on top to add a new E820 type so > we can look up the region later, which removes some > redundant code in the i915 driver at least. > > Any comments? I assume no one likes this, but maybe it's > just another early quirk we'll have to live with... No strong feelings against it - my only suggestion would be to make this more visible - right now it's added as e820 reserved which hides amongst other areas already marked reserved - would a low-key printk() of the range added make it more apparent that a kernel quirk activated here? Just so that people know that it came from the kernel, not the firmware. But in any case: Acked-by: Ingo Molnar <mingo@kernel.org> Thanks, Ingo ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: Ugly patches for stolen reservation 2013-07-25 20:05 ` Ugly patches for stolen reservation Ingo Molnar @ 2013-07-25 20:16 ` Jesse Barnes 2013-07-25 22:10 ` [Intel-gfx] " Daniel Vetter 2013-07-25 22:42 ` H. Peter Anvin 1 sibling, 1 reply; 20+ messages in thread From: Jesse Barnes @ 2013-07-25 20:16 UTC (permalink / raw) To: Ingo Molnar; +Cc: intel-gfx, linux-kernel, hpa, mingo, tglx, torvalds On Thu, 25 Jul 2013 22:05:51 +0200 Ingo Molnar <mingo@kernel.org> wrote: > > * Jesse Barnes <jbarnes@virtuousgeek.org> wrote: > > > Patch 2/2 has the description, but suffice it to say I'm > > not really pleased with this, though it does solve a > > problem we have. On some machines, we get MMIO space > > allocated on top of this hidden memory, which can cause > > problems. I'm not sure if there are similar problems for > > other hunks of the address space; if so it's possible > > this could be made more general (though the bits for > > looking up the address of this region are definitely > > Intel graphics specific). > > It looks pretty hardware specific. Discovering it the hard > way and marking it e820 reserved in an early quirk is what > the firmware should have done to begin with - and I doubt > the kernel could do anything significantly cleaner. > > How does Windows manage to not crash? By luckily never > allocating PCI resources on top of the RAM? Or does it have > a quirk? Pretty sure Windows doesn't allocate MMIO space the same way we do, so doesn't run into this on platforms where it's not E820_RESERVED. On top of that, BIOS vendors probably just move things around until Windows boots and the device manager doesn't have any dreaded "yellow bang" icons that would prevent them from getting their "designed for windows" sticker. > > Chris has some patches on top to add a new E820 type so > > we can look up the region later, which removes some > > redundant code in the i915 driver at least. > > > > Any comments? I assume no one likes this, but maybe it's > > just another early quirk we'll have to live with... > > No strong feelings against it - my only suggestion would be > to make this more visible - right now it's added as e820 > reserved which hides amongst other areas already marked > reserved - would a low-key printk() of the range added make > it more apparent that a kernel quirk activated here? Sounds good, I think Chris's patches should satisfy there. They make it a new E820 type so it's clear in /proc/iomem too. > > Just so that people know that it came from the kernel, not > the firmware. > > But in any case: > > Acked-by: Ingo Molnar <mingo@kernel.org> Thanks, -- Jesse Barnes, Intel Open Source Technology Center ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Intel-gfx] Ugly patches for stolen reservation 2013-07-25 20:16 ` Jesse Barnes @ 2013-07-25 22:10 ` Daniel Vetter 0 siblings, 0 replies; 20+ messages in thread From: Daniel Vetter @ 2013-07-25 22:10 UTC (permalink / raw) To: Jesse Barnes Cc: Ingo Molnar, intel-gfx, linux-kernel, hpa, mingo, torvalds, tglx On Thu, Jul 25, 2013 at 01:16:48PM -0700, Jesse Barnes wrote: > On Thu, 25 Jul 2013 22:05:51 +0200 > Ingo Molnar <mingo@kernel.org> wrote: > > > Chris has some patches on top to add a new E820 type so > > > we can look up the region later, which removes some > > > redundant code in the i915 driver at least. > > > > > > Any comments? I assume no one likes this, but maybe it's > > > just another early quirk we'll have to live with... > > > > No strong feelings against it - my only suggestion would be > > to make this more visible - right now it's added as e820 > > reserved which hides amongst other areas already marked > > reserved - would a low-key printk() of the range added make > > it more apparent that a kernel quirk activated here? > > Sounds good, I think Chris's patches should satisfy there. They make > it a new E820 type so it's clear in /proc/iomem too. I think it'd be good to get it all in in one go, since with Chris' patches i915 will also use the detection logic from the quirk code, so more testing for it. And imo the i915 cleanups shouldn't conflict really with ongoing work in drm/i915, so could all go in through x86 trees. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: Ugly patches for stolen reservation 2013-07-25 20:05 ` Ugly patches for stolen reservation Ingo Molnar 2013-07-25 20:16 ` Jesse Barnes @ 2013-07-25 22:42 ` H. Peter Anvin 2013-07-25 23:17 ` Jesse Barnes 2013-07-26 0:31 ` Linus Torvalds 1 sibling, 2 replies; 20+ messages in thread From: H. Peter Anvin @ 2013-07-25 22:42 UTC (permalink / raw) To: Ingo Molnar, Jesse Barnes; +Cc: intel-gfx, linux-kernel, mingo, tglx, torvalds So the bootloader is just as likely to step on things... what happens when/if it does? Ingo Molnar <mingo@kernel.org> wrote: > >* Jesse Barnes <jbarnes@virtuousgeek.org> wrote: > >> Patch 2/2 has the description, but suffice it to say I'm >> not really pleased with this, though it does solve a >> problem we have. On some machines, we get MMIO space >> allocated on top of this hidden memory, which can cause >> problems. I'm not sure if there are similar problems for >> other hunks of the address space; if so it's possible >> this could be made more general (though the bits for >> looking up the address of this region are definitely >> Intel graphics specific). > >It looks pretty hardware specific. Discovering it the hard >way and marking it e820 reserved in an early quirk is what >the firmware should have done to begin with - and I doubt >the kernel could do anything significantly cleaner. > >How does Windows manage to not crash? By luckily never >allocating PCI resources on top of the RAM? Or does it have >a quirk? > >> Chris has some patches on top to add a new E820 type so >> we can look up the region later, which removes some >> redundant code in the i915 driver at least. >> >> Any comments? I assume no one likes this, but maybe it's >> just another early quirk we'll have to live with... > >No strong feelings against it - my only suggestion would be >to make this more visible - right now it's added as e820 >reserved which hides amongst other areas already marked >reserved - would a low-key printk() of the range added make >it more apparent that a kernel quirk activated here? > >Just so that people know that it came from the kernel, not >the firmware. > >But in any case: > >Acked-by: Ingo Molnar <mingo@kernel.org> > >Thanks, > > Ingo -- Sent from my mobile phone. Please excuse brevity and lack of formatting. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: Ugly patches for stolen reservation 2013-07-25 22:42 ` H. Peter Anvin @ 2013-07-25 23:17 ` Jesse Barnes 2013-07-26 0:49 ` H. Peter Anvin 2013-07-26 0:31 ` Linus Torvalds 1 sibling, 1 reply; 20+ messages in thread From: Jesse Barnes @ 2013-07-25 23:17 UTC (permalink / raw) To: H. Peter Anvin Cc: Ingo Molnar, intel-gfx, linux-kernel, mingo, tglx, torvalds Well, it's ok if the boot loader writes to this memory, the worst that'll happen is you'll see garbage on the screen. If the boot loader tries to do MMIO mapping on top it'll get into trouble... but why would it do that? Jesse On Thu, 25 Jul 2013 15:42:25 -0700 "H. Peter Anvin" <hpa@zytor.com> wrote: > So the bootloader is just as likely to step on things... what happens when/if it does? > > Ingo Molnar <mingo@kernel.org> wrote: > > > >* Jesse Barnes <jbarnes@virtuousgeek.org> wrote: > > > >> Patch 2/2 has the description, but suffice it to say I'm > >> not really pleased with this, though it does solve a > >> problem we have. On some machines, we get MMIO space > >> allocated on top of this hidden memory, which can cause > >> problems. I'm not sure if there are similar problems for > >> other hunks of the address space; if so it's possible > >> this could be made more general (though the bits for > >> looking up the address of this region are definitely > >> Intel graphics specific). > > > >It looks pretty hardware specific. Discovering it the hard > >way and marking it e820 reserved in an early quirk is what > >the firmware should have done to begin with - and I doubt > >the kernel could do anything significantly cleaner. > > > >How does Windows manage to not crash? By luckily never > >allocating PCI resources on top of the RAM? Or does it have > >a quirk? > > > >> Chris has some patches on top to add a new E820 type so > >> we can look up the region later, which removes some > >> redundant code in the i915 driver at least. > >> > >> Any comments? I assume no one likes this, but maybe it's > >> just another early quirk we'll have to live with... > > > >No strong feelings against it - my only suggestion would be > >to make this more visible - right now it's added as e820 > >reserved which hides amongst other areas already marked > >reserved - would a low-key printk() of the range added make > >it more apparent that a kernel quirk activated here? > > > >Just so that people know that it came from the kernel, not > >the firmware. > > > >But in any case: > > > >Acked-by: Ingo Molnar <mingo@kernel.org> > > > >Thanks, > > > > Ingo > -- Jesse Barnes, Intel Open Source Technology Center ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: Ugly patches for stolen reservation 2013-07-25 23:17 ` Jesse Barnes @ 2013-07-26 0:49 ` H. Peter Anvin 0 siblings, 0 replies; 20+ messages in thread From: H. Peter Anvin @ 2013-07-26 0:49 UTC (permalink / raw) To: Jesse Barnes; +Cc: Ingo Molnar, intel-gfx, linux-kernel, mingo, tglx, torvalds On 07/25/2013 04:17 PM, Jesse Barnes wrote: > Well, it's ok if the boot loader writes to this memory, the worst > that'll happen is you'll see garbage on the screen. If the boot loader > tries to do MMIO mapping on top it'll get into trouble... but why would > it do that? > > Jesse Much worse: it could be hunting for a place to put the kernel, and put it there. -hpa ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: Ugly patches for stolen reservation 2013-07-25 22:42 ` H. Peter Anvin 2013-07-25 23:17 ` Jesse Barnes @ 2013-07-26 0:31 ` Linus Torvalds 2013-07-26 0:48 ` H. Peter Anvin 2013-07-26 15:51 ` Jesse Barnes 1 sibling, 2 replies; 20+ messages in thread From: Linus Torvalds @ 2013-07-26 0:31 UTC (permalink / raw) To: H. Peter Anvin Cc: Ingo Molnar, Jesse Barnes, intel-gfx, Linux Kernel Mailing List, Ingo Molnar, Thomas Gleixner On Thu, Jul 25, 2013 at 3:42 PM, H. Peter Anvin <hpa@zytor.com> wrote: > So the bootloader is just as likely to step on things... what happens when/if it does? This isn't a new problem. We've had this "firmware tables don't show all devices" issue before. The only odd thing about this one is how the quirk in question uses "e820_add_region()" instead of just adding things to the MMIO list. And I think that's actually likely a mistake. So Jesse, why don't you do what the other quirks do, and claim an actual MMIO resource? If you make it a real resource, you'll get to use fancy things like REAL NAMES, and actually document it. With human-readable strings. See quirk_io_region() in drivers/pci/quirks.c for example. The same code except for IORESOURCE_MEM should do a lovely job.. And even *if* it's already marked reserved in the e820 table, it just looks nice in /proc/iomem. Hmm? Linus ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: Ugly patches for stolen reservation 2013-07-26 0:31 ` Linus Torvalds @ 2013-07-26 0:48 ` H. Peter Anvin 2013-07-26 17:52 ` [Intel-gfx] " Daniel Vetter 2013-07-26 15:51 ` Jesse Barnes 1 sibling, 1 reply; 20+ messages in thread From: H. Peter Anvin @ 2013-07-26 0:48 UTC (permalink / raw) To: Linus Torvalds Cc: Ingo Molnar, Jesse Barnes, intel-gfx, Linux Kernel Mailing List, Ingo Molnar, Thomas Gleixner On 07/25/2013 05:31 PM, Linus Torvalds wrote: > On Thu, Jul 25, 2013 at 3:42 PM, H. Peter Anvin <hpa@zytor.com> wrote: >> So the bootloader is just as likely to step on things... what happens when/if it does? > > This isn't a new problem. We've had this "firmware tables don't show > all devices" issue before. > Yes, I just want to know what happens. > The only odd thing about this one is how the quirk in question uses > "e820_add_region()" instead of just adding things to the MMIO list. > And I think that's actually likely a mistake. > > So Jesse, why don't you do what the other quirks do, and claim an > actual MMIO resource? If you make it a real resource, you'll get to > use fancy things like REAL NAMES, and actually document it. With > human-readable strings. > > See quirk_io_region() in drivers/pci/quirks.c for example. The same > code except for IORESOURCE_MEM should do a lovely job.. > > And even *if* it's already marked reserved in the e820 table, it just > looks nice in /proc/iomem. We should do both -- mark it reserved in early boot, and add it as an MMIO region later during boot. The problem here, if I'm reading this right, is that this memory region is marked as normal RAM in e820, which is much worse than just not marking it as reserved; we need to intercept this memory before we genuinely turn it into normal RAM. At the same time, we have no protection against the bootloader using this as memory or even placing the kernel there. The BIOS needs to be fixed regardless of what workarounds we do in the kernel. -hpa ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Intel-gfx] Ugly patches for stolen reservation 2013-07-26 0:48 ` H. Peter Anvin @ 2013-07-26 17:52 ` Daniel Vetter 0 siblings, 0 replies; 20+ messages in thread From: Daniel Vetter @ 2013-07-26 17:52 UTC (permalink / raw) To: H. Peter Anvin Cc: Linus Torvalds, intel-gfx, Linux Kernel Mailing List, Ingo Molnar, Ingo Molnar, Thomas Gleixner On Thu, Jul 25, 2013 at 05:48:42PM -0700, H. Peter Anvin wrote: > On 07/25/2013 05:31 PM, Linus Torvalds wrote: > > On Thu, Jul 25, 2013 at 3:42 PM, H. Peter Anvin <hpa@zytor.com> wrote: > >> So the bootloader is just as likely to step on things... what happens when/if it does? > > > > This isn't a new problem. We've had this "firmware tables don't show > > all devices" issue before. > > > > Yes, I just want to know what happens. > > > The only odd thing about this one is how the quirk in question uses > > "e820_add_region()" instead of just adding things to the MMIO list. > > And I think that's actually likely a mistake. > > > > So Jesse, why don't you do what the other quirks do, and claim an > > actual MMIO resource? If you make it a real resource, you'll get to > > use fancy things like REAL NAMES, and actually document it. With > > human-readable strings. > > > > See quirk_io_region() in drivers/pci/quirks.c for example. The same > > code except for IORESOURCE_MEM should do a lovely job.. > > > > And even *if* it's already marked reserved in the e820 table, it just > > looks nice in /proc/iomem. > > We should do both -- mark it reserved in early boot, and add it as an > MMIO region later during boot. We started to reserve this range in drm/i915 (and still do), that's why we've noticed that something is amiss. > > The problem here, if I'm reading this right, is that this memory region > is marked as normal RAM in e820, which is much worse than just not > marking it as reserved; we need to intercept this memory before we > genuinely turn it into normal RAM. I haven't seen a case where it's marked as ram, but many systems don't mark it as reserved. Then the pci code can go around and put a bar into the middle of that range. The other issue (benign but ugly) is that the RAM buffer (used to protect stolen memory without knowing where exactly it is because it's not properly reserved in the e820 map) sometimes ends up in the graphics stolen range. With the new iomeme reserve code in drm/i915 we've stopped using stolen in such cases to avoid surprises, but that kills a few neat features. Thus far we haven't tracked down any bugs to such overlaps yet, but otoh we don't use stolen all that much yet. But we have plans to put it to real use (and use the full range) and I'd like to get the reservation conflicts sorted out before we do so. > At the same time, we have no protection against the bootloader using > this as memory or even placing the kernel there. The BIOS needs to be > fixed regardless of what workarounds we do in the kernel. Like I've said we haven't seen it earmarked as ram anywhere yet, so I don't think this is something we need to concern ousrselves with. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: Ugly patches for stolen reservation 2013-07-26 0:31 ` Linus Torvalds 2013-07-26 0:48 ` H. Peter Anvin @ 2013-07-26 15:51 ` Jesse Barnes 1 sibling, 0 replies; 20+ messages in thread From: Jesse Barnes @ 2013-07-26 15:51 UTC (permalink / raw) To: Linus Torvalds Cc: H. Peter Anvin, Ingo Molnar, intel-gfx, Linux Kernel Mailing List, Ingo Molnar, Thomas Gleixner On Thu, 25 Jul 2013 17:31:29 -0700 Linus Torvalds <torvalds@linux-foundation.org> wrote: > On Thu, Jul 25, 2013 at 3:42 PM, H. Peter Anvin <hpa@zytor.com> wrote: > > So the bootloader is just as likely to step on things... what happens when/if it does? > > This isn't a new problem. We've had this "firmware tables don't show > all devices" issue before. > > The only odd thing about this one is how the quirk in question uses > "e820_add_region()" instead of just adding things to the MMIO list. > And I think that's actually likely a mistake. > > So Jesse, why don't you do what the other quirks do, and claim an > actual MMIO resource? If you make it a real resource, you'll get to > use fancy things like REAL NAMES, and actually document it. With > human-readable strings. > > See quirk_io_region() in drivers/pci/quirks.c for example. The same > code except for IORESOURCE_MEM should do a lovely job.. > > And even *if* it's already marked reserved in the e820 table, it just > looks nice in /proc/iomem. > > Hmm? I should have mentioned yesterday that we *do* try to reserve the resource in our driver init, with pretty name and everything. The issue here is making sure we are actually *able* to reserve it without conflicts at driver init time. If the PCI layer has put something there (misc MMIO or the "RAM buffer" intended to prevent stuff like this) because it's not listed in the E820 map, we'll fail to get at this memory in our driver init. Thus the early reservation in early-quirks, followed by a real request_mem_region later on. Doing the request_mem_region before the PCI layer gets its hands on it isn't really possible, because __request_region depends on the memory allocator being initialized. So rather than add a new hook elsewhere in setup_arch or start_kernel I figured I'd use an early quirk. Reasonable? Note iomem in both cases. Before (RAM buffer prevents our allocation): cafff000-caffffff : System RAM cb000000-cbffffff : RAM buffer cfa00000-feafffff : PCI Bus 0000:00 d0000000-dfffffff : 0000:00:02.0 After (yay): cb000000-cb9fffff : RAM buffer cba00000-cf9fffff : reserved cba00000-cf9fffff : Graphics Stolen Memory cfa00000-feafffff : PCI Bus 0000:00 Thanks, -- Jesse Barnes, Intel Open Source Technology Center ^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2013-07-26 17:52 UTC | newest] Thread overview: 20+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-07-25 16:37 Ugly patches for stolen reservation Jesse Barnes 2013-07-25 16:37 ` [PATCH 1/2] drm/i915: split PCI IDs out into i915_drm.h v3 Jesse Barnes 2013-07-25 23:06 ` Chris Wilson 2013-07-25 16:37 ` [PATCH 2/2] x86: add early quirk for reserving Intel graphics stolen memory v3 Jesse Barnes 2013-07-25 22:42 ` Chris Wilson 2013-07-25 22:59 ` Chris Wilson 2013-07-25 23:18 ` Jesse Barnes 2013-07-26 8:10 ` [Intel-gfx] " Daniel Vetter 2013-07-26 16:58 ` H. Peter Anvin 2013-07-26 17:12 ` Jesse Barnes 2013-07-25 20:05 ` Ugly patches for stolen reservation Ingo Molnar 2013-07-25 20:16 ` Jesse Barnes 2013-07-25 22:10 ` [Intel-gfx] " Daniel Vetter 2013-07-25 22:42 ` H. Peter Anvin 2013-07-25 23:17 ` Jesse Barnes 2013-07-26 0:49 ` H. Peter Anvin 2013-07-26 0:31 ` Linus Torvalds 2013-07-26 0:48 ` H. Peter Anvin 2013-07-26 17:52 ` [Intel-gfx] " Daniel Vetter 2013-07-26 15:51 ` Jesse Barnes
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox