Re: [PATCH] drm/i915: make i915_stolen_to_physical() return phys_addr_t

From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Chris Wilson <chris@chris-wilson.co.uk>,
	Paulo Zanoni <paulo.r.zanoni@intel.com>,
	intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/i915: make i915_stolen_to_physical() return phys_addr_t
Date: Fri, 27 Jan 2017 17:06:53 +0200	[thread overview]
Message-ID: <20170127150653.GC31595@intel.com> (raw)
In-Reply-To: <20170127144240.GE24154@nuc-i3427.alporthouse.com>

On Fri, Jan 27, 2017 at 02:42:40PM +0000, Chris Wilson wrote:
> On Fri, Jan 27, 2017 at 04:38:47PM +0200, Ville Syrjälä wrote:
> > On Fri, Jan 27, 2017 at 02:20:52PM +0000, Chris Wilson wrote:
> > > On Fri, Jan 27, 2017 at 03:59:19PM +0200, Ville Syrjälä wrote:
> > > > On Thu, Jan 26, 2017 at 06:19:07PM -0200, Paulo Zanoni wrote:
> > > > > The i915_stolen_to_physical() function has 'unsigned long' as its
> > > > > return type but it returns the 'base' variable, which is of type
> > > > > 'u32'. The only place where this function is called assigns the
> > > > > returned value to dev_priv->mm.stolen_base, which is of type
> > > > > 'phys_addr_t'. The return value is actually a physical address and
> > > > > everything else in the stolen memory code seems to be using
> > > > > phys_addr_t, so fix i915_stolen_to_physical() to use phys_addr_t.
> > > > 
> > > > Size of phys_addr_t depends on PAE no? So what if someone were to boot
> > > > a !PAE kernel on a machine where stolen lives somewhere >4GiB?
> > > 
> > > dma_addr_t should be correct there, right? And in effect we do regard
> > > this as only dma accessible, so the white lie would have some nice
> > > semantic benefits.
> > 
> > config ARCH_PHYS_ADDR_T_64BIT
> >         def_bool y
> >         depends on X86_64 || X86_PAE
> > 
> > config ARCH_DMA_ADDR_T_64BIT
> >         def_bool y
> >         depends on X86_64 || HIGHMEM64G
> > 
> > So looks like the size of dma_addr_t also depends on the config.
> 
> We are dependent upon dma_addr_t (for transporting the addresses to the
> GTT), so use it and stick a warn or build bug if it ever comes up short?

Needs a runtime check since the address comes from the firmware.
But I guess we could just check whether it'll fit, and if not we
simply don't use stolen?

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx