From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH 1/3] drm: Add colouring to the range allocator Date: Tue, 10 Jul 2012 11:40:14 +0200 Message-ID: <20120710094014.GC5108@phenom.ffwll.local> References: <1341833679-11614-1-git-send-email-chris@chris-wilson.co.uk> <1341833679-11614-2-git-send-email-chris@chris-wilson.co.uk> <20120710092157.GB5108@phenom.ffwll.local> <1341912549_237368@CP5-2952> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-we0-f177.google.com (mail-we0-f177.google.com [74.125.82.177]) by gabe.freedesktop.org (Postfix) with ESMTP id 73ABCA0946 for ; Tue, 10 Jul 2012 02:40:14 -0700 (PDT) Received: by weyr3 with SMTP id r3so193663wey.36 for ; Tue, 10 Jul 2012 02:40:13 -0700 (PDT) Content-Disposition: inline In-Reply-To: <1341912549_237368@CP5-2952> 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: Benjamin Herrenschmidt , intel-gfx@lists.freedesktop.org, Jerome Glisse , Ben Skeggs , Daniel Vetter , Alex Deucher List-Id: intel-gfx@lists.freedesktop.org On Tue, Jul 10, 2012 at 10:29:09AM +0100, Chris Wilson wrote: > On Tue, 10 Jul 2012 11:21:57 +0200, Daniel Vetter wrote: > > On Mon, Jul 09, 2012 at 12:34:37PM +0100, Chris Wilson wrote: > > > In order to support snoopable memory on non-LLC architectures (so that > > > we can bind vgem objects into the i915 GATT for example), we have to > > > avoid the prefetcher on the GPU from crossing memory domains and so > > > prevent allocation of a snoopable PTE immediately following an uncached > > > PTE. To do that, we need to extend the range allocator with support for > > > tracking and segregating different node colours. > > > > > > This will be used by i915 to segregate memory domains within the GTT. > > > > > > Signed-off-by: Chris Wilson > > > Cc: Dave Airlie > > Cc: Benjamin Herrenschmidt > > > Cc: Ben Skeggs > > > Cc: Jerome Glisse > > > Cc: Alex Deucher > > > Cc: Daniel Vetter > > > > Two little bikesheds: > > - Do we really need 64bits of colour? Especially since we have quite a few > > bits of space left ... > > It was following the convention that we passed around an argument large > enough to stuff a pointer into if we ever needed to make a far more > complex decision. I think the right thing to do in that case would be to embed the gtt_space and do an upcast ;-) > > - I think we could add a new insert_color helper that always takes a range > > (we can select the right rang in the driver). That way this patch > > wouldn't need to touch the drivers, and we could take the opportunity to > > embed the gtt_space mm_node into our gem object ... > > I was just a bit more wary of adding yet another helper since they > quickly get just as confusing as the extra arguments they replace. :) Oh, I guess you mean a different helper than I do. I think we should add a new drm_mm_insert_node_colour function that takes a pre-allocated mm_node, color and range and goes hole-hunting. That way we'd avoid changing any of the existing drivers (who rather likely will never care about). And I wouldn't have to convert over the drm_mm functions that deal with pre-allocated drm_mm_node structs when I get around to resurrect the embedd gtt_space patch. I agree that shoveling all the alignment constrains into a new helper for would be a bit too much overkill -Daniel -- Daniel Vetter Mail: daniel@ffwll.ch Mobile: +41 (0)79 365 57 48