| Message ID | 1362718720-27048-2-git-send-email-yinghai@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Mar 07, 2013 at 08:58:27PM -0800, Yinghai Lu wrote: > diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c > index 69d97cb..7f9380b 100644 > --- a/drivers/gpu/drm/i915/i915_gem_stolen.c > +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c > @@ -81,7 +81,7 @@ static unsigned long i915_stolen_to_physical(struct drm_device *dev) > base -= dev_priv->mm.gtt->stolen_size; > } else { > /* Stolen is immediately above Top of Memory */ > - base = max_low_pfn_mapped << PAGE_SHIFT; > + base = __REMOVED_CRAZY__ << PAGE_SHIFT; Huh?
On Thu, Mar 7, 2013 at 9:10 PM, Tejun Heo <tj@kernel.org> wrote: > On Thu, Mar 07, 2013 at 08:58:27PM -0800, Yinghai Lu wrote: >> diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c >> index 69d97cb..7f9380b 100644 >> --- a/drivers/gpu/drm/i915/i915_gem_stolen.c >> +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c >> @@ -81,7 +81,7 @@ static unsigned long i915_stolen_to_physical(struct drm_device *dev) >> base -= dev_priv->mm.gtt->stolen_size; >> } else { >> /* Stolen is immediately above Top of Memory */ >> - base = max_low_pfn_mapped << PAGE_SHIFT; >> + base = __REMOVED_CRAZY__ << PAGE_SHIFT; > > Huh? Whole function: 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; /* On the machines I have tested the Graphics Base of 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 >= 6) { /* Read Base Data of Stolen Memory Register (BDSM) directly. * Note that there is also a MCHBAR miror at 0x1080c0 or * we could use device 2:0x5c instead. */ pci_read_config_dword(pdev, 0xB0, &base); base &= ~4095; /* lower bits used for locking register */ } else if (INTEL_INFO(dev)->gen > 3 || IS_G33(dev)) { /* Read Graphics Base of Stolen Memory directly */ pci_read_config_dword(pdev, 0xA4, &base); #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; } else { /* Stolen is immediately above Top of Memory */ base = __REMOVED_CRAZY__ << PAGE_SHIFT; #endif } return base; }
On Thu, Mar 7, 2013 at 9:22 PM, Yinghai Lu <yinghai@kernel.org> wrote: > On Thu, Mar 7, 2013 at 9:10 PM, Tejun Heo <tj@kernel.org> wrote: >> On Thu, Mar 07, 2013 at 08:58:27PM -0800, Yinghai Lu wrote: >>> diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c >>> index 69d97cb..7f9380b 100644 >>> --- a/drivers/gpu/drm/i915/i915_gem_stolen.c >>> +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c >>> @@ -81,7 +81,7 @@ static unsigned long i915_stolen_to_physical(struct drm_device *dev) >>> base -= dev_priv->mm.gtt->stolen_size; >>> } else { >>> /* Stolen is immediately above Top of Memory */ >>> - base = max_low_pfn_mapped << PAGE_SHIFT; >>> + base = __REMOVED_CRAZY__ << PAGE_SHIFT; >> >> Huh? > > Whole function: Yeah, but can't we still just do 1LLU << 32 like other places? Or at least explain what was there before? It's gonna confuse the hell out of future readers of the code.
On Thu, Mar 7, 2013 at 9:25 PM, Tejun Heo <tj@kernel.org> wrote: > On Thu, Mar 7, 2013 at 9:22 PM, Yinghai Lu <yinghai@kernel.org> wrote: >> On Thu, Mar 7, 2013 at 9:10 PM, Tejun Heo <tj@kernel.org> wrote: >>> On Thu, Mar 07, 2013 at 08:58:27PM -0800, Yinghai Lu wrote: >>>> diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c >>>> index 69d97cb..7f9380b 100644 >>>> --- a/drivers/gpu/drm/i915/i915_gem_stolen.c >>>> +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c >>>> @@ -81,7 +81,7 @@ static unsigned long i915_stolen_to_physical(struct drm_device *dev) >>>> base -= dev_priv->mm.gtt->stolen_size; >>>> } else { >>>> /* Stolen is immediately above Top of Memory */ >>>> - base = max_low_pfn_mapped << PAGE_SHIFT; >>>> + base = __REMOVED_CRAZY__ << PAGE_SHIFT; >>> >>> Huh? >> >> Whole function: > > Yeah, but can't we still just do 1LLU << 32 like other places? Or at > least explain what was there before? It's gonna confuse the hell out > of future readers of the code. They are not using memblock_find_in_range(), so 1ULL<< will not help. Really hope i915 drm guys could clean that hacks. Thanks Yinghai
On Thu, Mar 7, 2013 at 9:27 PM, Yinghai Lu <yinghai@kernel.org> wrote: > They are not using memblock_find_in_range(), so 1ULL<< will not help. > > Really hope i915 drm guys could clean that hacks. The code isn't being used. Just leave it alone. Maybe add a comment. The change is just making things more confusing.
On 03/07/2013 09:28 PM, Tejun Heo wrote: > On Thu, Mar 7, 2013 at 9:27 PM, Yinghai Lu <yinghai@kernel.org> wrote: >> They are not using memblock_find_in_range(), so 1ULL<< will not help. >> >> Really hope i915 drm guys could clean that hacks. > > The code isn't being used. Just leave it alone. Maybe add a comment. > The change is just making things more confusing. > Indeed, but... Daniel: can you guys clean this up or can we just remove the #if 0 clause? -hpa
On Thu, Mar 07, 2013 at 10:09:26PM -0800, H. Peter Anvin wrote: > On 03/07/2013 09:28 PM, Tejun Heo wrote: > > On Thu, Mar 7, 2013 at 9:27 PM, Yinghai Lu <yinghai@kernel.org> wrote: > >> They are not using memblock_find_in_range(), so 1ULL<< will not help. > >> > >> Really hope i915 drm guys could clean that hacks. > > > > The code isn't being used. Just leave it alone. Maybe add a comment. > > The change is just making things more confusing. > > > > Indeed, but... > > Daniel: can you guys clean this up or can we just remove the #if 0 clause? I guess we could just put this into a comment explaining where stolen memory for the gfx devices is at on gen2. But tbh I don't mind if we just keep the #if 0 code around. For all newer platforms we can get at that offset through mch bar registers, so I don't really care. -Daniel
On Mon, Mar 11, 2013 at 11:50:48PM +0100, Daniel Vetter wrote: > On Thu, Mar 07, 2013 at 10:09:26PM -0800, H. Peter Anvin wrote: > > On 03/07/2013 09:28 PM, Tejun Heo wrote: > > > On Thu, Mar 7, 2013 at 9:27 PM, Yinghai Lu <yinghai@kernel.org> wrote: > > >> They are not using memblock_find_in_range(), so 1ULL<< will not help. > > >> > > >> Really hope i915 drm guys could clean that hacks. > > > > > > The code isn't being used. Just leave it alone. Maybe add a comment. > > > The change is just making things more confusing. > > > > > > > Indeed, but... > > > > Daniel: can you guys clean this up or can we just remove the #if 0 clause? > > I guess we could just put this into a comment explaining where stolen > memory for the gfx devices is at on gen2. But tbh I don't mind if we just > keep the #if 0 code around. For all newer platforms we can get at that > offset through mch bar registers, so I don't really care. If you want to keep the comment accurate s/max_low_pfn_mapped/max_pfn_mapped/ as the machines in question don't support more than 4GiB anyway. Or you can help address the underlying issue of figuring out how we can derive the location of the stolen memory which is reserved by the BIOS but not communicated to the OS. -Chris
The problem is that the code will be broken, and so it makes no sense. The #if 0 is really confusing. Daniel Vetter <daniel@ffwll.ch> wrote: >On Thu, Mar 07, 2013 at 10:09:26PM -0800, H. Peter Anvin wrote: >> On 03/07/2013 09:28 PM, Tejun Heo wrote: >> > On Thu, Mar 7, 2013 at 9:27 PM, Yinghai Lu <yinghai@kernel.org> >wrote: >> >> They are not using memblock_find_in_range(), so 1ULL<< will not >help. >> >> >> >> Really hope i915 drm guys could clean that hacks. >> > >> > The code isn't being used. Just leave it alone. Maybe add a >comment. >> > The change is just making things more confusing. >> > >> >> Indeed, but... >> >> Daniel: can you guys clean this up or can we just remove the #if 0 >clause? > >I guess we could just put this into a comment explaining where stolen >memory for the gfx devices is at on gen2. But tbh I don't mind if we >just >keep the #if 0 code around. For all newer platforms we can get at that >offset through mch bar registers, so I don't really care. >-Daniel
diff --git a/arch/x86/include/asm/page_types.h b/arch/x86/include/asm/page_types.h index 54c9787..b012b82 100644 --- a/arch/x86/include/asm/page_types.h +++ b/arch/x86/include/asm/page_types.h @@ -43,7 +43,6 @@ extern int devmem_is_allowed(unsigned long pagenr); -extern unsigned long max_low_pfn_mapped; extern unsigned long max_pfn_mapped; static inline phys_addr_t get_max_mapped(void) diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c index 90d8cc9..4dcaae7 100644 --- a/arch/x86/kernel/setup.c +++ b/arch/x86/kernel/setup.c @@ -113,13 +113,11 @@ #include <asm/prom.h> /* - * max_low_pfn_mapped: highest direct mapped pfn under 4GB - * max_pfn_mapped: highest direct mapped pfn over 4GB + * max_pfn_mapped: highest direct mapped pfn * * The direct mapping only covers E820_RAM regions, so the ranges and gaps are * represented by pfn_mapped */ -unsigned long max_low_pfn_mapped; unsigned long max_pfn_mapped; #ifdef CONFIG_DMI diff --git a/arch/x86/mm/init.c b/arch/x86/mm/init.c index 59b7fc4..abcc241 100644 --- a/arch/x86/mm/init.c +++ b/arch/x86/mm/init.c @@ -313,10 +313,6 @@ static void add_pfn_range_mapped(unsigned long start_pfn, unsigned long end_pfn) nr_pfn_mapped = clean_sort_range(pfn_mapped, E820_X_MAX); max_pfn_mapped = max(max_pfn_mapped, end_pfn); - - if (start_pfn < (1UL<<(32-PAGE_SHIFT))) - max_low_pfn_mapped = max(max_low_pfn_mapped, - min(end_pfn, 1UL<<(32-PAGE_SHIFT))); } bool pfn_range_is_mapped(unsigned long start_pfn, unsigned long end_pfn) diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c index 586e7e9..c9e36d7 100644 --- a/drivers/acpi/osl.c +++ b/drivers/acpi/osl.c @@ -624,9 +624,12 @@ void __init acpi_initrd_override(void *data, size_t size) if (table_nr == 0) return; - acpi_tables_addr = - memblock_find_in_range(0, max_low_pfn_mapped << PAGE_SHIFT, - all_tables_size, PAGE_SIZE); + /* under 4G at first, then above 4G */ + acpi_tables_addr = memblock_find_in_range(0, 1ULL<<32, + all_tables_size, PAGE_SIZE); + if (!acpi_tables_addr) + acpi_tables_addr = memblock_find_in_range(1ULL<<32, -1ULL, + all_tables_size, PAGE_SIZE); if (!acpi_tables_addr) { WARN_ON(1); return; diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c index 69d97cb..7f9380b 100644 --- a/drivers/gpu/drm/i915/i915_gem_stolen.c +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c @@ -81,7 +81,7 @@ static unsigned long i915_stolen_to_physical(struct drm_device *dev) base -= dev_priv->mm.gtt->stolen_size; } else { /* Stolen is immediately above Top of Memory */ - base = max_low_pfn_mapped << PAGE_SHIFT; + base = __REMOVED_CRAZY__ << PAGE_SHIFT; #endif }
Now we have arch_pfn_mapped array, and max_low_pfn_mapped should not be used anymore. Only user is ACPI_OVERRIDE, and it should not use that, as later accessing is using early_remap. Change to try to 4G below and then 4G above. Other user is in drm/i915, but it is commented out. Should use arch_pfn_mapped or just 1<<(32-PAGE_SHIFT) instead. Suggested-by: H. Peter Anvin <hpa@linux.intel.com> Signed-off-by: Yinghai Lu <yinghai@kernel.org> Cc: Thomas Renninger <trenn@suse.de> Cc: "Rafael J. Wysocki" <rjw@sisk.pl> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Cc: David Airlie <airlied@linux.ie> Cc: Jacob Shin <jacob.shin@amd.com> Cc: linux-acpi@vger.kernel.org Cc: dri-devel@lists.freedesktop.org --- arch/x86/include/asm/page_types.h | 1 - arch/x86/kernel/setup.c | 4 +--- arch/x86/mm/init.c | 4 ---- drivers/acpi/osl.c | 9 ++++++--- drivers/gpu/drm/i915/i915_gem_stolen.c | 2 +- 5 files changed, 8 insertions(+), 12 deletions(-)