From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jesse Barnes Subject: Re: [PATCH] drm/i915: BIOS and power context stolen mem handling for VLV v6 Date: Tue, 7 May 2013 15:32:22 -0700 Message-ID: <20130507153222.5f1e3dd8@jbarnes-desktop> References: <1367959066-2381-1-git-send-email-jbarnes@virtuousgeek.org> <1367962471-3534-1-git-send-email-jbarnes@virtuousgeek.org> <20130507215736.GA22739@cantiga.alporthouse.com> <20130507150842.4f246176@jbarnes-desktop> <20130507221440.GB22739@cantiga.alporthouse.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from oproxy6-pub.bluehost.com (oproxy6-pub.bluehost.com [67.222.54.6]) by gabe.freedesktop.org (Postfix) with SMTP id B603EE5C1A for ; Tue, 7 May 2013 15:31:31 -0700 (PDT) In-Reply-To: <20130507221440.GB22739@cantiga.alporthouse.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: Chris Wilson Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Tue, 7 May 2013 23:14:40 +0100 Chris Wilson wrote: > On Tue, May 07, 2013 at 03:08:42PM -0700, Jesse Barnes wrote: > > On Tue, 7 May 2013 22:57:37 +0100 > > Chris Wilson wrote: > > > > > On Tue, May 07, 2013 at 02:34:31PM -0700, Jesse Barnes wrote: > > > > +static void valleyview_setup_pctx(struct drm_device *dev) > > > > +{ > > > > + struct drm_i915_private *dev_priv = dev->dev_private; > > > > + struct drm_i915_gem_object *pctx; > > > > + unsigned long pctx_paddr; > > > > + u32 pcbr; > > > > + int pctx_size = 24*1024; > > > > + > > > > + pcbr = I915_READ(VLV_PCBR); > > > > + if (pcbr) { > > > > + /* BIOS set it up already, grab the pre-alloc'd space */ > > > > + int pcbr_offset; > > > > + > > > > + pcbr_offset = (pcbr & (~4095)) - dev_priv->mm.stolen_base; > > > > + pctx = i915_gem_object_create_stolen_for_preallocated(dev_priv->dev, > > > > + pcbr_offset, > > > > + pcbr_offset, > > > > + pctx_size); > > > > > > We're reserving global GTT space here for someting that just looks like > > > it requires contiguous physical memory. This may cause issues if > > > something else is already bound to that GTT range. I think we want to > > > extend the interface to not reserve GTT space if gtt_offset == -1. > > > > I should have added a comment for this too. The physical range for the > > power context must reside in stolen space. So while it needs > > contiguous physical memory, it also has a relationship to the stolen > > handling. I'd prefer to see a failure here than to try to do something > > fancy with funky GTT vs stolen setups... (Still waiting on the BIOS > > guys to promise the identity map going forward.) > > Hmm, I am not understanding exactly what you need here. The register you > are reading looks to be a physical address - so why do we need to also > reserve a virtual address with the same offset? And if that is required, > shouldn't the else branch also reserve the corresponding virtual > address for its stolen allocation? I thought the stolen create code would do that for me; on looking I see I'm wrong. I'm conflating the stolen physically contiguous range here with the first stolen_size portion of the GTT space. I don't know if anything depends on that or not, but given the way things are set up at boot time I thought it would be best not to break that assumption. -- Jesse Barnes, Intel Open Source Technology Center